Peter Krempa [Mon, 12 Aug 2019 11:48:51 +0000 (13:48 +0200)]
qemu: snapshot: Restrict file existence check only for local storage
Soon we'll allow more protocols and storage types with snapshots where
we in some cases can't check whether the storage already exists.
Restrict the sanity checks whether the destination images exist or not
for local storage where it's easy. For any other case we will fail
later.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Fri, 9 Aug 2019 13:15:07 +0000 (15:15 +0200)]
qemu: snapshot: Don't modify persistent XML if disk source is different
While the VM is running the persistent source of a disk might differ
e.g. as the 'newDef' was redefined. Our snapshot code would blindly
rewrite the source of such disk if it shared the 'target'. Fix this by
checking whether the source is the same in the first place.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Thu, 15 Aug 2019 17:29:43 +0000 (19:29 +0200)]
util: storagefile: Flag backing store strings with authentication
Using inline authentication for storage volumes will not work properly
as libvirt requires use of the secret driver for the auth data and
thus would not be able to represent the passwords stored in the backing
store string.
Make sure that the backing store parsers return 1 which is a sign for
the caller to not use the file in certain cases.
The test data include iscsi via a json pseudo-protocol string and URIs
with the userinfo part being present.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Fri, 16 Aug 2019 09:34:27 +0000 (11:34 +0200)]
util: storagefile: Don't traverse storage sources unusable by VM
virStorageFileGetMetadataRecurse would include files in the backing
chain which would not really be usable by libvirt directly e.g.
when such file would be promoted to the top layer by an active block
commit as for example inline authentication data can't be represented in
the VM xml file. The idea is to use secrets for this.
With the changes to the backing store string parsers we can report and
propagate if such a thing is present in the configuration and thus start
skipping those files in the backing chain traversal code. This approach
still allows to report the appropriate backing store string in the
storage driver which doesn't directly use the backing file.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Fri, 16 Aug 2019 09:28:03 +0000 (11:28 +0200)]
util: storagefile: Clarify docs for '@report_broken' of virStorageFileGetMetadata
virStorageFileGetMetadata does not report error if we can't interrogate
the file somehow. Clarify this in the description of the @report_broken
flag as it implies we should report an error in that case. The problem
is that we don't know whether there's a problem and unfortunately just
offload it to qemu.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Fri, 16 Aug 2019 09:14:31 +0000 (11:14 +0200)]
util: storagefile: Add handling of unusable storage sources
Introduce new semantics to virStorageSourceNewFromBacking and some
of the helpers used by it which propagate the return value from the
callers.
The new return value introduced by this patch allows to notify the
calller that the parsed virStorageSource correctly describes the source
but contains data such as inline authentication which libvirt does not
want to support directly. This means that such file would e.g. unusable
as a storage source (e.g. when actively commiting the overlay to it) or
would not work with blockdev.
The caller will then be able to decide whether to consider this backing
file as viable or just fall back to qemu dealing with it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Thu, 15 Aug 2019 14:43:40 +0000 (16:43 +0200)]
util: storagefile: Modify arguments of virStorageSourceNewFromBackingAbsolue
Return the parsed storage source via an pointer in arguments and return
an integer from the function. Describe the semantics with a comment for
the function and adjust callers to the new semantics.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Thu, 15 Aug 2019 12:49:49 +0000 (14:49 +0200)]
util: storage: Modify return value of virStorageSourceNewFromBacking
Return the storage source definition via a pointer in the arguments and
document the returned values. This will simplify the possibility to
ignore certain backing store types which are not representable by
libvirt.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Fri, 16 Aug 2019 15:01:10 +0000 (17:01 +0200)]
qemu: alias: Generate 'qomName' of disk with useraliases
Commit fb64e176f4f forgot to delete the check that short-circuits the
disk alias creation if the alias is already present. The side effect
of this is that the creation qomName which is necessary to be able to
refer to disk frontends when -blockdev is used was skipped when user
aliases are used.
Fix it by deleting the check. Also prevent any potential memory leaks
from calling this function repeatedly by creating the qomName only when
it's not present.
Michal Privoznik [Fri, 23 Aug 2019 11:37:25 +0000 (13:37 +0200)]
virsh: Allow graceful console shutdown
Currently, whenever there's a regular EOF on the console stream
or an error the virStreamAbort() is called regardless. While this
may not actually break anything, we should call virStreamFinish()
to let the daemon know we've successfully received all the data
and are shutting down the stream gracefully.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Roman Bolshakov <r.bolshakov@yadro.com>
Michal Privoznik [Fri, 23 Aug 2019 13:11:20 +0000 (15:11 +0200)]
storage_driver: Don't crash in storagePoolCreateXML
In my recent patches I've introduced
virStoragePoolObjIsStarting() which is then used to protect
storage pool definition when the pool object is locked and
unlocked during long running jobs. Well, my patches did not
anticipate that @obj can be NULL under 'cleanup' label in
storagePoolCreateXML() (for instance when parsing XML fails).
This imperfection is causing libvirtd to crash then.
Fixes: 13284a6b83 storage_driver: Protect pool def during startup and build Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Roman Bolshakov [Wed, 21 Aug 2019 13:33:18 +0000 (16:33 +0300)]
tools: console: Relax stream EOF handling
Regular VM shutdown triggers the error for existing session of virsh
console and it returns with non-zero exit code:
error: internal error: console stream EOF
The message and status code are misleading because there's no real
error. virStreamRecv returns 0 correctly when EOF is reached.
Existing implementations of esx, fd, and remote streams behave the same
for virStreamFinish and virStreamAbort: they close the stream. So, we
can continue to use virStreamAbort to handle EOF and errors from
virStreamRecv but additonally we can report error if virStreamAbort
fails.
Fixes: 29f2b5248c6 ("tools: console: pass stream/fd errors to user") Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Roman Bolshakov [Wed, 21 Aug 2019 16:13:23 +0000 (19:13 +0300)]
tests: Make references to global symbols indirect in test drivers
A library has to be built with -flat_namespace to get all references to
global symbols indirected. That can also be achieved with two-level
namespace interposition but we're not using explicit symbol
interposition since it's more verbose and requires massive changes to
the mocks.
This provides a way to interpose a mock for virQEMUCapsProbeHostCPU from
qemucpumock and fixes domaincapstest on macOS.
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
Roman Bolshakov [Wed, 21 Aug 2019 16:13:22 +0000 (19:13 +0300)]
tests: Avoid gnulib replacements in mocks
gnulib headers change stat, lstat and open to replacement functions,
even for function definitions. This effectively disables standard
library overrides in virfilewrapper and virmockstathelpers since they
are never reached.
Rename the functions and provide a declartion that uses correct
assembler name for the mocks.
This fixes firmware lookup in domaincapstest on macOS.
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
Roman Bolshakov [Wed, 21 Aug 2019 16:13:21 +0000 (19:13 +0300)]
tests: Use flat namespace on macOS
Test executables and mocks have assumption that any symbol can be
replaced with LD_PRELOAD. That's not a case for macOS unless flat
namespace is used, because every external symbol reference records the
library to be looked up. And the symbols cannot be replaced unless dyld
interposing is used.
Setting DYLD_FORCE_FLAT_NAMESPACE changes symbol lookup behaviour to be
similar to Linux dynamic linker. It's more lightweight solution than
explicitly decorating all mock symbols as interpositions and building
libvirt as interposable dynamic library.
This fixes vircryptotest and allows to proceed other tests that rely on
mocks a little bit further.
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
Roman Bolshakov [Wed, 21 Aug 2019 16:13:20 +0000 (19:13 +0300)]
tests: Lookup extended stat/lstat in mocks
macOS syscall interface (/usr/lib/system/libsystem_kernel.dylib) has
three kinds of stat but only one of them can be used to fill
"struct stat": stat$INODE64.
virmockstathelpers looks up regular stat instead of stat$INODE64. That
causes a failure in qemufirmwaretest because "struct stat" is laid out
differently from the values returned by stat.
Introduce VIR_MOCK_REAL_INIT_ALIASED that can be used to lookup
stat$INODE64 and lstat$INODE64 and use it to setup real functions on
macOS.
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
Roman Bolshakov [Wed, 21 Aug 2019 16:13:19 +0000 (19:13 +0300)]
build: Use flat namespace for libvirt on macOS
>From ld(1):
By default all references resolved to a dynamic library record the
library to which they were resolved. At runtime, dyld uses that
information to directly resolve symbols. The alternative is to use the
-flat_namespace option. With flat namespace, the library is not
recorded. At runtime, dyld will search each dynamic library in load
order when resolving symbols. This is slower, but more like how other
operating systems resolve symbols.
That fixes the set of tests that preload a mock library to replace
library symbols:
qemublocktest
qemumonitorjsontest
viriscsitest
virmacmaptest
virnetserverclienttest
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
Roman Bolshakov [Wed, 21 Aug 2019 16:13:18 +0000 (19:13 +0300)]
tests: Drop /private CWD prefix in commandhelper
/tmp is a symbolic link to /private/tmp on macOS. That causes failures
in commandtest, because getcwd returns /private/tmp and the expected
output doesn't match to "CWD: /tmp".
Rathern than making a copy of commanddata solely for macOS, the /private
prefix is stripped.
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
Roman Bolshakov [Wed, 21 Aug 2019 16:13:17 +0000 (19:13 +0300)]
tests: Remove -module flag for mocks
macOS has two kinds of loadable libraries: MH_BUNDLE, and MH_DYLIB.
bundle is used for plugins that are loaded with dlopen/dlsym/dlclose.
And there's no way to preload a bundle into an application. dynamic
linker (dyld) will reject it when finds it in DYLD_INSERT_LIBRARIES.
Unfortunately, a bundle is built if -module flag is provided to libtool.
The flag has been removed to build dylibs with ".dylib" suffix.
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
Roman Bolshakov [Wed, 21 Aug 2019 16:13:16 +0000 (19:13 +0300)]
tests: Add lib- prefix to all mocks
In preparation libtool "-module" flag removal, add lib prefix to all
mock shared objects.
While at it, introduce VIR_TEST_MOCK macros that makes path out of mock
name to be used with VIR_TEST_PRELOAD or VIR_TEST_MAIN_PRELOAD. That,
hopefully, improves readability, reduces line length and allows to
tailor VIR_TEST_MOCK for specific platform if it has shared library
suffix different from ".so".
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
Roman Bolshakov [Wed, 21 Aug 2019 16:13:15 +0000 (19:13 +0300)]
tests: Preload mocks with DYLD_INSERT_LIBRARIES on macOS
LD_PRELOAD has no effect on macOS. Instead, dyld(1) provides a way for
symbol hooking via DYLD_INSERT_LIBRARIES. The variable should contain
colon-separated paths to the dylibs to be inserted.
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
Roman Bolshakov [Wed, 21 Aug 2019 16:13:14 +0000 (19:13 +0300)]
tests: Avoid IPv4-translated IPv6 address in sockettest
getnameinfo on macOS formats certain IPv6 addresses as IPv4-translated
addresses. The following pattern has been observed:
::ffff is formated as ::0.0.255.255
::fffe is formated as ::0.0.255.254
::ffff:0 is formated as ::255.255.0.0
::fffe:0 is formated as ::255.254.0.0
::ffff:0:0 is formated as ::ffff:0.0.0.0
::fffe:0:0 is formated as ::fffe:0:0
::ffff:0:0:0 is formated as ::ffff:0:0:0
The getnameinfo behavior causes a failure for:
DO_TEST_PARSE_AND_FORMAT("::ffff", AF_UNSPEC, true);
Use non-ambigious IPv6 for parse/format testing.
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
Roman Bolshakov [Wed, 21 Aug 2019 16:13:13 +0000 (19:13 +0300)]
tests: Don't test octal localhost IP in sockettest on macOS
getaddrinfo on macOS doesn't interpret octal IPv4 addresses. Only
inet_aton can be used for that. Therefore, from macOS standpoint
"0177.0.0.01" is not the same as "127.0.0.1".
The issue was also discovered by python and dotnet core:
https://bugs.python.org/issue27612
https://github.com/dotnet/corefx/issues/8362
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
After my previous patches we have virPCIDeviceBindToStub() and
virPCIDeviceUnbindFromStub() which really do nothing but call
virPCIDeviceBindToStubWithOverride() and
virPCIDeviceUnbindFromStubWithOverride() respectively.
Drop "WithOverride" from the names and drop the thin wrappers.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Michal Privoznik [Tue, 20 Aug 2019 14:22:08 +0000 (16:22 +0200)]
news: Document KVM assignment removal
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Michal Privoznik [Tue, 20 Aug 2019 11:52:55 +0000 (13:52 +0200)]
virpcimock: Drop @driverActions enum
This enum was introduced to model how RHEL-7 kernel behaves - for
some reason going with the old way (via new_id + bind) fails but
using driver_override succeeds. Well, we don't need to care about
that anymore since we don't create new_id file.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Michal Privoznik [Tue, 20 Aug 2019 11:49:54 +0000 (13:49 +0200)]
virpcimock: Don't create new_id or remove_id files
Now that PCI attach/detach happens solely via driver_override
these two files are no longer needed.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Michal Privoznik [Tue, 20 Aug 2019 11:31:27 +0000 (13:31 +0200)]
virpcimock: Don't create "pci-stub" driver
Now that nothing supports "pci-stub" driver (aka KVM style of PCI
device assignment) there is no need for virpcimock to create it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Michal Privoznik [Tue, 20 Aug 2019 11:17:44 +0000 (13:17 +0200)]
virpci: Drop newid style of PCI device detach
As stated in 84f9358b18346 all kernels that we are interested in
have 'drivers_override'. Drop the other, older style of
overriding PCI device driver - newid.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Michal Privoznik [Mon, 19 Aug 2019 11:44:15 +0000 (13:44 +0200)]
virpci: Remove unused virPCIDeviceWaitForCleanup
This function is no longer used after previous commit.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Michal Privoznik [Mon, 19 Aug 2019 10:01:47 +0000 (12:01 +0200)]
virpci: Drop 'pci-stub' driver
Now that no one uses KVM style of PCI assignment we can safely
remove 'pci-stub' backend.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Michal Privoznik [Mon, 19 Aug 2019 09:47:19 +0000 (11:47 +0200)]
virhostdev: Disable legacy kvm assignment
The KVM assignment is going to be removed shortly. Don't let the
hostdev module configure it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Michal Privoznik [Tue, 20 Aug 2019 10:13:49 +0000 (12:13 +0200)]
qemu: Drop unused qemuOpenPCIConfig()
After previous commits, the function is not used anymore.
Remove it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Michal Privoznik [Mon, 19 Aug 2019 09:04:05 +0000 (11:04 +0200)]
virhostdev: Unify virDomainHostdevDef to virPCIDevice translation
There are two places where we need to create virPCIDevice from
given virDomainHostdevDef. In both places the code is duplicated.
Move them into a single function and call it from those two
places.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Michal Privoznik [Tue, 20 Aug 2019 07:27:44 +0000 (09:27 +0200)]
tests: Remove 'kvm' PCI backend from domaincapstest
The KVM assignment was removed in qemu driver in previous commit.
Remove it from domaincapstest too which is hard coding it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>
KVM style of PCI devices assignment was dropped in kernel in
favor of vfio pci (see kernel commit v4.12-rc1~68^2~65). Since
vfio is around for quite some time now and is far superior
discourage people in using KVM style.
Ideally, I'd make QEMU_CAPS_VFIO_PCI implicitly assumed but turns
out qemu-3.0.0 doesn't support vfio-pci device for RISC-V.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Starting up or building some types of pools may take a very long
time (e.g. a misconfigured NFS). Holding the pool object locked
throughout the whole time hurts concurrency, e.g. if there's
another thread that is listing all the pools.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Fri, 24 May 2019 14:35:46 +0000 (16:35 +0200)]
storage_driver: Protect pool def during startup and build
In near future the storage pool object lock will be released
during startPool and buildPool callback (in some backends). But
this means that another thread may acquire the pool object lock
and change its definition rendering the former thread access not
only stale definition but also access freed memory
(virStoragePoolObjAssignDef() will free old def when setting a
new one).
One way out of this would be to have the pool appear as active
because our code deals with obj->def and obj->newdef just fine.
But we can't declare a pool as active if it's not started or
still building up. Therefore, have a boolean flag that is very
similar and forces virStoragePoolObjAssignDef() to store new
definition in obj->newdef even for an inactive pool. In turn, we
have to move the definition to correct place when unsetting the
flag. But that's as easy as calling
virStoragePoolUpdateInactive().
Technically speaking, change made to
storageDriverAutostartCallback() is not needed because until
storage driver is initialized no storage API can run therefore
there can't be anyone wanting to change the pool's definition.
But I'm doing the change there for consistency anyways.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Fri, 24 May 2019 14:35:45 +0000 (16:35 +0200)]
storagePoolCreateXML: Don't lose persistent storage on failed create
If there's a persistent storage and user tries to start a new one
with the same name and UUID (e.g. to test new configuration) it
may happen that upon failure we lose the persistent defintion.
Fortunately, we don't remove it from the disk only from the
internal list of the pools.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Fri, 24 May 2019 14:35:44 +0000 (16:35 +0200)]
virstorageobj: Introduce VIR_STORAGE_POOL_OBJ_LIST_ADD_LIVE flag
This flag can be used to denote that the definition we're trying
to assign to a pool object is live definition and thus the
inactive definition should be saved into ->newDef.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
There is no need for this function to call
virStoragePoolObjEndAPI(). The object is perfectly usable after
return from this function. In fact, all callers will call
virStoragePoolObjEndAPI() eventually.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Fri, 24 May 2019 14:35:38 +0000 (16:35 +0200)]
virStoragePoolObjListForEach: Grab a reference for pool object
Turns out there's one callback that might remove a storage pool
during its run: storagePoolUpdateAllState() call
storagePoolUpdateStateCallback() which may call
virStoragePoolUpdateInactive() which in turn may call
virStoragePoolObjRemove(). Problem is that the
UpdateStateCallback() sees a storage pool object with just two
references: one for each hash table holding the object. If the
function ends up calling ObjRemove() then upon removing the
object from hash tables those references are gone and thus any
subsequent call touching the object is invalid.
The solution to this problem is to grab reference for the object
we are running iterator with.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Fri, 24 May 2019 14:35:37 +0000 (16:35 +0200)]
virStoragePoolObjRemove: Don't unlock pool object upon return
The fact that we're removing a pool object from the list of pools
doesn't mean we want to unlock it. It violates locking policy
too as object locking and unlocking is not done on the same
level.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
It may happen that we leave some XATTRs behind. For instance, on
a sudden power loss, the host just shuts down without calling
restore on domain paths. This creates a problem, because when the
host starts up again, the XATTRs are there but they don't reflect
the true state and this may result in libvirt denying start of a
domain.
To solve this, save a unique timestamp (host boot time) among
with our XATTRs.
Michal Privoznik [Wed, 21 Aug 2019 08:46:27 +0000 (10:46 +0200)]
security: Don't increase XATTRs refcounter on failure
If user has two domains, each have the same disk (configured for
RW) but each runs with different seclabel then we deny start of
the second domain because in order to do that we would need to
relabel the disk but that would cut the first domain off. Even if
we did not do that, qemu would fail to start because it would be
unable to lock the disk image for the second time. So far, this
behaviour is expected. But what is not expected is that we
increase the refcounter in XATTRs and leave it like that.
What happens is that when the second domain starts,
virSecuritySetRememberedLabel() is called, and since there are
XATTRs from the first domain it increments the refcounter and
returns it (refcounter == 2 at this point). Then callers
(virSecurityDACSetOwnership() and
virSecuritySELinuxSetFileconHelper()) realize that refcounter is
greater than 1 and desired seclabel doesn't match the one the
disk image already has and an error is produced. But the
refcounter is never decremented.
virUUIDFormat: s/VIR_UUID_RAW_LEN/VIR_UUID_BUFLEN/ in comment
The function takes raw UUID and formats it into string
representation. However, the comment mistakenly states that the
expected size of raw UUID buffer is VIR_UUID_RAW_LEN bytes. We
don't have such constant since v0.3.2~24. It should have been
VIR_UUID_BUFLEN.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Andrea Bolognani [Thu, 15 Aug 2019 18:56:24 +0000 (20:56 +0200)]
ci: Stop using --workdir
Now that we're using sudo, the initial work directory is no
longer relevant since the user will find themselves in their
home directory when they get control anyway.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Andrea Bolognani [Thu, 15 Aug 2019 13:37:38 +0000 (15:37 +0200)]
ci: Run $(CI_PREPARE_SCRIPT) as root
In order for the prepare script to be really useful, it needs
to be able to perform privileged operations such as installing
additional packages or setting up custom mount points.
In order to achieve that, we now run the container as root,
run the prepare script with full privilege, and only then
switch to the unprivileged account with sudo.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Andrea Bolognani [Thu, 15 Aug 2019 13:23:23 +0000 (15:23 +0200)]
ci: Generalize running commands inside the container
Both for ci-build and ci-shell we want to execute basically
the same setup and cleanup logic, the only difference being
that for the former we then run the build script and with the
latter a shell.
Rework the targets so that they both call the generic
ci-run-command rule passing an appropriate $(CI_COMMAND).
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Andrea Bolognani [Thu, 15 Aug 2019 12:28:17 +0000 (14:28 +0200)]
ci: Introduce $(CI_BUILD_SCRIPT)
Instead of hardcoding build instructions into the Makefile,
move them to a separate script that's mounted into the
container.
This gives us a couple of advantages: we no longer have to
deal with the awkward quoting required when embedding shell
code in a Makefile, and we also provide the users with a way
to override the default build instructions with their own.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Andrea Bolognani [Thu, 15 Aug 2019 18:52:07 +0000 (20:52 +0200)]
ci: Move source directory under $(CI_USER_HOME)
Now that we have a home directory for the user, storing the
source there rather than in a custom top-level directory is
the obvious choice.
Later on we're also going to add some more files related to
builds, and storing everything in the user's home directory
will keep things nice and tidy.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Andrea Bolognani [Thu, 15 Aug 2019 18:34:20 +0000 (20:34 +0200)]
ci: Create user's home directory in the container
Some applications expect the user's home directory to be
present on the system and require workarounds when that's not
the case. Creating the home directory along with everything
else is easy enough for us, so let's just do that.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Andrea Bolognani [Thu, 15 Aug 2019 12:06:14 +0000 (14:06 +0200)]
ci: Move everything to a separate directory
We're going to have a few more CI-related files in a second, and
it makes sense to have a separate directory for them rather than
littering the root directory.
$(CI_SCRATCHDIR) can now also be created inside the CI directory,
and as a bonus the make rune necessary to start CI builds without
running configure first becomes shorter.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Ján Tomko [Tue, 20 Aug 2019 20:14:13 +0000 (22:14 +0200)]
util: xml: introduce virXMLNamespaceRegister
A wrapper around xmlXPathRegisterNs that will save us
from having to include xpathInternals.h everywhere
we want to use a custom namespace and open-coding
the strings already contained in virXMLNamespace.
Signed-off-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Ján Tomko [Tue, 20 Aug 2019 22:03:15 +0000 (00:03 +0200)]
xml: virXMLNamespace: add prefix
We have hardcoded the namespace prefix in various places:
1) the xmlns string stored in the 'href' function
2) the xmlXPathRegisterNs call in each parser
3) all the parsing and formatting code actually dealing
with these elements
While eliminating the third one is probably a job for an
actual XML-aware formatter, let's store the prefix separately
here in the virXMLNamespace structure so that future patches
can get rid of the first two bullets.
Signed-off-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
projects / libvirt.git / log