John Ferlan [Mon, 15 Feb 2016 21:55:08 +0000 (16:55 -0500)]
qemu: Make basic upfront checks before create command
Create qemuBuildCommandLineValidate to make some checks before trying
to build the command. This will move some logic from much later to much
earlier - we shouldn't be adjusting any data so that shouldn't matter.
Peter Krempa [Tue, 28 Jul 2015 13:34:58 +0000 (15:34 +0200)]
virsh: Remove <backingStore> when changing cdrom media source
Since the code is changing the source image path by modifying the
existing XML snippet the <backingStore> stays in place.
As <backingStore> is relevant to the <source> part of the image, the
update of that part makes the element invalid.
CD/floppy images usually don't have a backing chain and the element is
currently ignored though but it might start being used in the future so
let's start behaving correctly.
Drop the <backingStore> subtree once we want to update the XML.
Before this patch, you'd get:
$ virsh change-media --eject --print-xml 10 hdc
<disk type="file" device="cdrom">
<driver name="qemu" type="qcow2"/>
Cole Robinson [Wed, 17 Feb 2016 14:10:28 +0000 (09:10 -0500)]
tests: Remove unused virtTestClearLineRegex
This was only used for test 'xml blanking', which has now all
been removed, and isn't an ideal paradigm anyways since it
inhibits easy XML regeneration.
This is not valid XML, as either a UUID or usage must be specified in
the secret block. It's not clear though how the argv2xml code can do
anything correct here, since XML like this requires a libvirt secret
object to have already been defined.
The current test suite handles this by blanking out any <secret> block
in the XML. This avoids domainschematest failures.
Instead of blanking, let's hardcode a usage= name. This lets us test
the other bits of generated <secret> XML, and is a step towards wiring
up VIR_TEST_REGENERATE_OUTPUT
Peter Krempa [Wed, 17 Feb 2016 12:10:11 +0000 (13:10 +0100)]
qemu: Remove unnecessary calculations in qemuDomainSaveMemory
Now that the file migration doesn't require us to use 'dd' and other
legacy stuff for too old qemus we don't even have to calcuate the
offsets and other stuff.
Michal Privoznik [Wed, 17 Feb 2016 13:25:35 +0000 (14:25 +0100)]
vircgroup: Update virCgroupDenyDevicePath stub
In cf113e8d we changed the declaration of
virCgroupAllowDevicePath() and virCgroupDenyDevicePath().
However, while updating the stub for non-cgroup platforms for the
former we forgot to update the latter too causing a build
failure.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
According to the original commit message, this is dead code:
It is highly unlikely that a backend will know how to create a
volume from a different volume (buildVolFrom) and not know how to
create an empty volume (createVol).
Erik Skultety [Fri, 14 Aug 2015 07:17:01 +0000 (09:17 +0200)]
admin: Introduce adminDaemonConnectListServers API
This API is merely a convenience API, i.e. when managing clients connected to
daemon's servers, we should know (convenience) which server the specific client
is connected to. This implies a client-side representation of a server along
with a basic API to let the administrating client know what servers are actually
available on the daemon.
Signed-off-by: Erik Skultety <eskultet@redhat.com> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Erik Skultety [Thu, 13 Aug 2015 14:20:27 +0000 (16:20 +0200)]
admin: Introduce virAdmServer structure
This is the key structure of all management operations performed on the
daemon/clients. An admin client needs to be able to identify
another client (either admin or non-privileged client) to perform an
action on it. This identification includes a server the client is
connected to, thus a client-side representation of a server is needed.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Erik Skultety [Mon, 10 Aug 2015 10:39:33 +0000 (12:39 +0200)]
admin: Move admin_server.{h,c} to admin.{h,c}
This change is merely because admin_server would contain all the code
from dispatchers and helpers to the actual APIs. Admin should have
similar structure to the daemon-side remote driver - dispatchers and
helpers in a separate module, APIs in a separate module.
Best viewed with -M.
Signed-off-by: Erik Skultety <eskultet@redhat.com> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Erik Skultety [Mon, 10 Aug 2015 11:01:44 +0000 (13:01 +0200)]
virnetdaemon: Store servers in a hash table
Since the daemon can manage and add (at fresh start) multiple servers,
we also should be able to add them from a JSON state file in case of a
daemon restart, so post exec restart support for multiple servers is also
provided. Patch also updates virnetdaemontest accordingly.
Signed-off-by: Erik Skultety <eskultet@redhat.com> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Erik Skultety [Fri, 12 Feb 2016 09:15:57 +0000 (10:15 +0100)]
util: Refactor virHashForEach so it returns as soon as an iterator fails
The method will now return 0 on success and -1 on error, rather than number of
items which it iterated over before it returned back to the caller. Since the
only place where we actually check the number of elements iterated is in
virhashtest, return value of 0 and -1 can be a pretty accurate hint that it
iterated over all the items. However, if we really want to know the number of
items iterated over (like virhashtest does), a counter has to be provided
through opaque data to each iterator call. This patch adjusts return value of
virHashForEach, refactors the body, so it returns as soon as one of the
iterators fail and adjusts virhashtest to reflect these changes.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Erik Skultety [Fri, 12 Feb 2016 09:03:50 +0000 (10:03 +0100)]
util: Add a return value to void hash iterators
Our existing virHashForEach method iterates through all items disregarding the
fact, that some of the iterators might have actually failed. Errors are usually
dispatched through an error element in opaque data which then causes the
original caller of virHashForEach to return -1. In that case, virHashForEach
could return as soon as one of the iterators fail. This patch changes the
iterator return type and adjusts all of its instances accordingly, so the
actual refactor of virHashForEach method can be dealt with later.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Peter Krempa [Tue, 16 Feb 2016 13:43:41 +0000 (14:43 +0100)]
util: cgroup: Allow ignoring EACCES in virCgroup(Allow|Deny)DevicePath
When adding disk images to ACL we may call those functions on NFS
shares. In that case we might get an EACCES, which isn't really relevant
since NFS would not hold a block device. This patch adds a flag that
allows to stop reporting an error on EACCES to avoid spaming logs.
Peter Krempa [Tue, 16 Feb 2016 12:57:10 +0000 (13:57 +0100)]
util: cgroup: Drop virCgroup(Allow|Deny)DeviceMajor
Since commit 47e5b5ae virCgroupAllowDevice allows to pass -1 as either
the minor or major device number and it automatically uses '*' in place
of that. Reuse the new approach through the code and drop the duplicated
functions.
Peter Krempa [Mon, 15 Feb 2016 16:17:02 +0000 (17:17 +0100)]
qemu: migration: Refactor code now that we assume support for fd migration
After removing capability check for fd migration the code that was left
behind didn't make quite sense. The old exec migration would be used in
case when pipe() failed. Remove the old code and make failure of pipe()
a hard error.
This additionally removes usage of virCgroupAllowDevicePath outside of
qemu_cgroup.c.
Andrea Bolognani [Tue, 16 Feb 2016 16:41:47 +0000 (17:41 +0100)]
conf: Use a temporary int variable to store GIC version
Since no value in the virGICVersion enumeration is negative, a clever
enough compiler can report an error such as
src/conf/domain_conf.c:15337:75: error: comparison of unsigned enum
expression < 0 is always false [-Werror,-Wtautological-compare]
if ((def->gic_version = virGICVersionTypeFromString(tmp)) < 0 ||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ ~
virGICVersionTypeFromString() can, however, return a negative value if
the input string is not part of the enumeration, so we definitely need
that check.
Work around the problem by storing the return value in a temporary int
variable.
John Ferlan [Mon, 15 Feb 2016 18:08:02 +0000 (13:08 -0500)]
qemu: Move qemuDomain*Address* functions
Create new modules qemu_domain_address.c and qemu_domain_address.h to
contain all the new functions and header data. Additionally move any
supporting static functions.
Make qemuDomainSupportsPCI non static.
Also, move and rename the following:
qemuSetSCSIControllerModel to qemuDomainSetSCSIControllerModel
qemuCollectPCIAddress to qemuDomainCollectPCIAddress
qemuValidateDevicePCISlotsPIIX3 to qemuDomainValidateDevicePCISlotsPIIX3
qemuAssignDevicePCISlots to qemuDomainAssignDevicePCISlots
John Ferlan [Mon, 15 Feb 2016 15:52:50 +0000 (10:52 -0500)]
qemu: Move qemuNetworkIfaceConnect to qemu_interface.c and rename
Move the misplaced function from qemu_command.c to qemu_interface.c
since it's closer in functionality there and had less to do with building
the command line.
Rename function to qemuInterfaceBridgeConnect and modify callers.
John Ferlan [Mon, 15 Feb 2016 15:26:40 +0000 (10:26 -0500)]
qemu: Move qemuPhysIfaceConnect to qemu_interface.c and rename
Move the misplaced function from qemu_command.c to qemu_interface.c
since it's closer in functionality there and had less to do with building
the command line.
Rename function to qemuInterfaceDirectConnect and modify callers.
Test all kinds of scenarios, including guests asking for GIC but
failing to specify a version, guests specifying an invalid version
and guests trying to use GIC with non-virt or even non-ARM machines.
Unify the naming to prepare for new test cases that will be added
later on.
Convert a couple of output XML files for the qemuxml2xml test to
symlinks while at it, since they were identical to the corresponding
input XML files anyways.
Moreover, since we're only interested in testing GIC support here,
simplify XML files by getting rid of the unrelevant bits.
This change allows to use "host" as a GIC version in the domain XML.
Since we'll need to update the virGICVersion enumeration to support
new GIC versions anyway, it makes sense to be a bit more strict in
the schema as well and reject values that are not in the enumeration.
We currently blindly accept any numeric value as a GIC version, even
though only GIC v2 and GIC v3 actually exist; on the other hand, we
reject "host", which is a perfectly legitimate value for QEMU guests.
This new enumeration contains all GIC versions libvirt is aware of.
Currently, on hot unplug of PCI devices with VFIO driver for QEMU, libvirt is
trying to restore the host devices to it's previous value (basically a chown
on the previous user/group).
However for devices with VFIO driver, when the device is unbinded it is
removed from the /dev/vfio file system causing the restore label to fail.
The fix is to not restore the label for those PCI devices since they are going
to be teared down anyway.
Signed-off-by: Ludovic Beliveau <ludovic.beliveau@windriver.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Peter Krempa [Fri, 12 Feb 2016 13:09:02 +0000 (14:09 +0100)]
vsh: Replace vshPrint macro with function
The macro would eat the first parameter. In some cases the format string
for vshPrint was eaten. In other cases the calls referenced variables
which did not exist in the given context. Avoid errors by doing compile
time checking.
Peter Krempa [Fri, 12 Feb 2016 13:00:28 +0000 (14:00 +0100)]
vsh: Simplify bailing out on OOM conditions
When we hit OOM it doesn't really make sense to format the error message
by attempting to allocate it. Introduce a simple helper that prints a
static message and terminates the execution.
Laine Stump [Thu, 11 Feb 2016 20:24:17 +0000 (15:24 -0500)]
util: clean up and expand 802.1QbX negotiation logging
The existing log messages for this have several problems; there are
two lines of log when one will suffice, they duplicate the function
name in log message (when it's already included by VIR_DEBUG), they're
missing some useful bits, they get logged even when the call is a NOP.
This patch cleans up the problems with those existing logs, and also
adds a new VIR_INFO-level log down at the function that is actually
creating and sending the netlink message that logs *everything* going
into the netlink message (which turns out to be much more useful in
practice for me; I didn't want to eliminate the logs at the existing
location though, in case they are useful in some scenario I'm
unfamiliar with; anyway those logs are remaining at debug level, so it
shouldn't be a bother to anyone).
Laine Stump [Tue, 9 Feb 2016 17:28:48 +0000 (12:28 -0500)]
network: consolidated info log for all network allocate/free operations
There are three functions that deal with allocating and freeing
devices from a networks netdev/pci device pool:
network(Allocate|Notify|Release)ActualDevice(). These functions also
maintain a counter of the number of domains currently using a network
(regardless of whether or not that network uses a device pool). Each
of these functions had multiple log messages (output using VIR_DEBUG)
that were in slightly different formats and gave varying amounts of
information.
This patch creates a single function to log the pertinent information
in a consistent manner for all three of these functions. Along with
assuring that all the functions produce a consistent form of output
(and making it simpler to change), it adds the MAC address of the
domain interface involved in the operation, making it possible to
verify which interface of which domain the operation is being done for
(assuming that all MAC addresses are unique, of course).
All of these messages are raised from DEBUG to INFO, since they don't
happen that often (once per interface per domain/libvirtd start or
domain stop), and can be very informative and helpful - eliminating
the need to log debug level messages makes it much easier to sort
these out.
Laine Stump [Thu, 11 Feb 2016 18:52:04 +0000 (13:52 -0500)]
network: consolidate connection count updates for device pool
networkReleaseActualDevice() and networkNotifyActualDevice() both were
updating the individual devices' connections count in two separate
places (unlike networkAllocateActualDevice() which does it in a single
unified place after success:). The code is correct, but prone to
confusion / later breakage. All of these updates are anyway located at
the end of if/else clauses that are (with the exception of a single
VIR_DEBUG() in each case) immediately followed by the success: label
anyway, so this patch replaces the duplicated ++/-- instructions with
a single ++/-- inside a qualifying "if (dev)" down below success:.
(NB: if dev != NULL, by definition we are using a device (either pci
or netdev, doesn't matter for these purposes) from the network's pool)
The VIR_DEBUG args (which will be replaced in a followup patch anyway)
were all adjusted to account for the connection count being out of
date at the time.
Andrea Bolognani [Wed, 10 Feb 2016 18:44:01 +0000 (19:44 +0100)]
domsuspend: Fix warning on mingw build
Commit d82170d introduced a workaround for domtop: in that example
program, we define a symbol called ERROR for our own use, but since
a symbol with the same name is already defined in one of mingw's
header files, we get a warning when using that compiler.
domsuspend defines the same problematic symbol, so the workaround
has been copied over.
Michal Privoznik [Wed, 10 Feb 2016 16:17:00 +0000 (17:17 +0100)]
storageVolCreateXMLFrom: Check if backend knows how to createVol
It is highly unlikely that a backend will know how to create a
volume from a different volume (buildVolFrom) and not know how to
create an empty volume (createVol). But:
1) we call the function without any prior check so if that's the
case we would SIGSEGV immediatelly
2) it's better to be safe than sorry.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Michal Privoznik [Wed, 10 Feb 2016 16:05:03 +0000 (17:05 +0100)]
storageVolCreateXML: Swap order of two operations
Firstly, we realloc internal list to hold new item (=volume that
will be potentially created) and then we check whether we
actually know how to create it. If we don't we consume more
memory than we really need for no good reason.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Michal Privoznik [Wed, 10 Feb 2016 15:48:15 +0000 (16:48 +0100)]
virsh: Teach vol-create-as to --print-xml
We have the same argument to many other commands that produce an
XML based on what user typed. But unfortunately vol-create-as
was missing it. Maybe nobody had needed it yet. Well, I did
just now.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Michal Privoznik [Wed, 10 Feb 2016 15:35:48 +0000 (16:35 +0100)]
cmdVolCreateAs: Rework to follow usual func pattern
The way we usually write functions is that we start the work and
if something goes bad we goto cleanup and roll back there. Or
just free resources that are no longer needed. Do the same here.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Michal Privoznik [Fri, 12 Feb 2016 12:48:02 +0000 (13:48 +0100)]
virportallocatortest: Run on linux only
After the rework of mocking of our tests there's the
virportallocator test failing to link on mingw. Well, it's the
mocking library actually:
../gnulib/lib/.libs/libgnu.a(bind.o): In function `rpl_bind':
/home/jenkins/libvirt-mingw/build32/gnulib/lib/../../../gnulib/lib/bind.c:33: multiple definition of `rpl_bind'
.libs/virportallocatormock_la-virportallocatormock.o:/home/jenkins/libvirt-mingw/build32/tests/../../tests/virportallocatormock.c:79: first defined here
I've no idea why this matters to mingw and does not to others.
Nevertheless, if we make the test linux only the problem goes
away.
Apparently, our test for RTLD_NEXT is not sufficient because
mingw32 defines it. Lets put aside for a while fact that it has
the same value as RTLD_DEFAULT which by description has different
meaning, shall we?
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
vz: fix race condition when adding domain to domains list
Race condition:
User calls defineXML to create new instance.
The main thread from vzDomainDefineXMLFlags() creates new instance by prlsdkCreateVm.
Then this thread calls prlsdkAddDomain to add new domain to domains list.
The second thread receives notification from hypervisor that new VM was created.
It calls prlsdkHandleVmAddedEvent() and also tries to add new domain to domains list.
These two threads call virDomainObjListFindByUUID() from prlsdkAddDomain() and don't find new domain.
So they add two domains with the same uuid to domains list.
This fix splits logic of prlsdkAddDomain() into two functions.
1. vzNewDomain() creates new empty domain in domains list with the specific uuid.
2. prlsdkLoadDomain() add data from VM to domain object.
New algorithm for creating an instance:
In vzDomainDefineXMLFlags() we add new domain to domain list by calling vzNewDomain()
and only after that we call CreateVm() to create VM.
It means that we "reserve" domain object with the specific uuid.
After creation of new VM we add info from this VM
to reserved domain object by calling prlsdkLoadDomain().
Before this patch prlsdkLoadDomain() worked in 2 different cases:
1. It creates and initializes new domain. Then updates it from sdk handle.
2. It updates existed domain from sdk handle.
In this patch we remove code which creates new domain from LoadDomain()
and move it to vzNewDomain().
Now prlsdkLoadDomain() only updates domain from skd handle.
In notification handler prlsdkHandleVmAddedEvent() we check
the existence of a domain and if it doesn't exist we add new domain by calling
vzNewDomain() and load info from sdk handle via prlsdkLoadDomain().
Bug cause:
Update the domain that is subscribed to hypervisor notification.
LoadDomain() rewrites notifications fields in vzDomObj structure and makes domain as "unsubscribed".
Fix:
Initialize notification fields in vzDomObj only if we create a new domain.
And do not reinitialize these fields if we update domain (by calling LoadDomain with olddom argument)
vz: make output arguments in prlsdkGetDomainIds as optional
prlsdkGetDomainIds() returns name and uuid for specified instance.
Now output arguments can be NULL.
It allows to get only necessary info(name or uuid).
The snapshot name generator truncates the original file name after a '.'
and replaces the suffix with the snapshot name. If two disks source
images differ only in the suffix portion, the generated name will be
duplicate.
Since this is a corner case just error out stating that a duplicate name
was generated. The user can work around this situation by providing
the file names explicitly.
Andrea Bolognani [Thu, 11 Feb 2016 10:33:45 +0000 (11:33 +0100)]
tests: Use plain close() in mock code
The virportallocatortest.c file is compiled both as a test case
and as a mock library; in the latter case, it can't use
VIR_FORCE_CLOSE() because mock libraries are not linked against
libvirt.
Replace VIR_FORCE_CLOSE() with plain close() to solve the issue.
Michal Privoznik [Thu, 11 Feb 2016 10:14:11 +0000 (11:14 +0100)]
dbus: Don't unref NULL messages
Apparently we are not the only ones with dumb free functions
because dbus_message_unref() does not accept NULL either. But if
I were to vote, this one is even more evil. Instead of returning
an error just like we do it immediately dereference any pointer
passed and thus crash you app. Well done DBus!
Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7f878ebda700 (LWP 31264)]
0x00007f87be4016e5 in ?? () from /usr/lib64/libdbus-1.so.3
(gdb) bt
#0 0x00007f87be4016e5 in ?? () from /usr/lib64/libdbus-1.so.3
#1 0x00007f87be3f004e in dbus_message_unref () from /usr/lib64/libdbus-1.so.3
#2 0x00007f87bf6ecf95 in virSystemdGetMachineNameByPID (pid=9849) at util/virsystemd.c:228
#3 0x00007f879761bd4d in qemuConnectCgroup (driver=0x7f87600a32a0, vm=0x7f87600c7550) at qemu/qemu_cgroup.c:909
#4 0x00007f87976386b7 in qemuProcessReconnect (opaque=0x7f87600db840) at qemu/qemu_process.c:3386
#5 0x00007f87bf6edfff in virThreadHelper (data=0x7f87600d5580) at util/virthread.c:206
#6 0x00007f87bb602334 in start_thread (arg=0x7f878ebda700) at pthread_create.c:333
#7 0x00007f87bb3481bd in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:109
(gdb) frame 2
#2 0x00007f87bf6ecf95 in virSystemdGetMachineNameByPID (pid=9849) at util/virsystemd.c:228
228 dbus_message_unref(reply);
(gdb) p reply
$1 = (DBusMessage *) 0x0
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Ján Tomko [Thu, 28 Jan 2016 16:44:33 +0000 (17:44 +0100)]
Clean up usage of 'ret' variable
Do not store the return value of called functions in the same variable
as the (future) return value of the current function.
This makes tracking the origin of the value easier and reduces
the chance of introducing a new point of exit without resetting
the return value back to -1.
Since we already have virtio channel events, we know when guest
agent within guest has (dis-)connected. Instead of us blindly
connecting to a socket that no one is listening to, we can just
follow what qemu-ga does. This has a nice benefit that we don't
need to 'guest-ping' the agent just to timeout and find out
nobody is listening.
The way that this commit is implemented:
- don't connect in qemuProcessLaunch directly, defer that to event
callback (which already follows the agent) -
processSerialChangedEvent
- after migration is settled, before we resume vCPUs, ask qemu
whether somebody is listening on the socket and if so, connect
to it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
John Ferlan [Tue, 9 Feb 2016 11:24:33 +0000 (06:24 -0500)]
qemu: Check return status for virDomainGraphicsListenSetAddress
Recent refactors in the vbox code to check the return status for the
function tipped Coverity's scales of justice for any functions that
do not check status - such as this one.
While I'm at it, since the call is essentially the same other than
whether starting from val or val+1 when val[0] = '[', just adjust
the val pointer by one and have one call instead of two.
Additionally, the call to virDomainGraphicsListenGetAddress is redundant
since it checking that the address field got filled. It's a leftover
from the strndup -> ListenSetAddress conversion (commit id 'ef79fb5b5')
John Ferlan [Tue, 9 Feb 2016 11:36:17 +0000 (06:36 -0500)]
qemu: Introduce qemuParseCommandLineVnc
Refactor qemuParseCommandLine to pull out the "-vnc" argument parsing
into its own helper function. Modify the code to use "cleanup" instead
of "error" and use the standard return processing to indicate success
or failure by using ret
Turns out that not linking against libvirt and gnulib is okay for
regular Linux (and FreeBSD) builds, but makes mingw very unhappy.
.../virnetserverclientmock_la-virnetserverclientmock.o:
In function `virNetSocketGetSELinuxContext':
.../virnetserverclientmock.c:61: undefined reference to `rpl_strdup'
.../libvirportallocatormock_la-virportallocatortest.o:
In function `init_syms':
.../virportallocatortest.c:61: undefined reference to `virFileClose'
projects / libvirt.git / log