Re: [libvirt] [PATCHv2] Ignore bridge template names with multiple printf conversions
On Wed, May 13, 2015 at 06:39:12AM -0400, John Ferlan wrote: On 05/05/2015 12:39 PM, Eric Blake wrote: On 05/05/2015 10:26 AM, Ján Tomko wrote: On Tue, May 05, 2015 at 10:14:18AM -0600, Eric Blake wrote: On 05/05/2015 10:05 AM, Ján Tomko wrote: For some reason, we allow a bridge name with %d in it, which we replace with an unsigned integer to form a bridge name that does not yet exist on the host. +if (def-bridge +(p = strchr(def-bridge, '%')) == strrchr(def-bridge, '%') +strstr(def-bridge, %d)) Simpler as: if (def-bridge strstr(def-bridge, %d) == strrchr(def-bridge, '%')) I still don't see it. [A] strchr(def-bridge, '%') [B] strrchr(def-bridge, '%') [C] strstr(def-bridge, %d)) When def-bridge is '%s%s%s%d', [A] points to the first %s, [B] points to the %d and so does [C] This string would pass the simplified condition (B == C), but not the full one (A != C) Okay, I see your counterargument. Still, strstr() is pretty expensive compared to just: if (def-bridge (p = strchr(def-bridge, '%')) == strrchr(def-bridge, '%') p[1] == 'd') Coverity complains : Event returned_null: strchr returns null (checked 273 out of 288 times). strchr does not return NULL here because networkFindUnusedBridgeName is only called if either def-bridge is NULL or def-bridge contains %d. Jan Event var_assigned: Assigning: p = null return value from strchr. Event cond_true: Condition (p = strchr(def-bridge, 37)) == strrchr(def-bridge, 37), taking true branch Event dereference:Dereferencing a null pointer p. John signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC v1 0/6] Live Migration with ephemeral host NIC devices
* Peter Krempa (pkre...@redhat.com) wrote: On Wed, May 13, 2015 at 09:08:39 +0100, Dr. David Alan Gilbert wrote: * Peter Krempa (pkre...@redhat.com) wrote: On Wed, May 13, 2015 at 11:36:26 +0800, Chen Fan wrote: my main goal is to add support migration with host NIC passthrough devices and keep the network connectivity. this series patch base on Shradha's patches on https://www.redhat.com/archives/libvir-list/2012-November/msg01324.html which is add migration support for host passthrough devices. 1) unplug the ephemeral devices before migration 2) do native migration 3) when migration finished, hotplug the ephemeral devices IMHO this algorithm is something that an upper layer management app should do. The device unplug operation is complex and it might not succeed which will make the current migration thread hang or fail in an intermediate state that will not be recoverable. However you wouldn't want each of the upper layer management apps implementing their own hacks for this; so something somewhere needs to standardise what the guest sees. The guest still will see an PCI device unplug request and will have to respond to it, then will be paused and after resume a new PCI device will appear. This is standardised. The nonstandardised part (which can't really be standardised) is how the bonding or other guest-dependant stuff will be handled, but that is up to the guest OS to handle. Why can't that be standardised? Don't we need to provide the information on what to bond to the guest and that this process is happening? The previous suggestion was to use guest-agent for this. From libvirt's perspective this is only something that will trigger the device unplug and plug the devices back. And there are a lot of issues here: 1) the destination of the migration might not have the desired devices This will trigger a lot of problems as we will not be able to guarantee that the devices reappear on the destination and if we'd wanted to check we'd need a new migration protocol AFAIK. But if it's using the bonding trick then that isn't fatal; it would still be able to have the bonded virtio device. 2) The guest OS might refuse to detach the PCI device (it might be stuck before PCI code is loaded) In that case the migration will be stuck forever and abort attempts will make the domain state basically undefined depending on the phase where it failed. Since we can't guarantee that the unplug of the PCI host devices will be atomic or that it will succeed we basically can't guarantee in any way in which state the VM will end up later after (a possibly failed) migration. To recover such state there are too many option that could be desired by the user that would be hard to implement in a way that would be flexible enough. I don't understand why this is any different to any other PCI device hot-unplug. Dave Peter -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC v1 0/6] Live Migration with ephemeral host NIC devices
On Wed, May 13, 2015 at 09:40:23AM +0100, Dr. David Alan Gilbert wrote: * Peter Krempa (pkre...@redhat.com) wrote: On Wed, May 13, 2015 at 09:08:39 +0100, Dr. David Alan Gilbert wrote: * Peter Krempa (pkre...@redhat.com) wrote: On Wed, May 13, 2015 at 11:36:26 +0800, Chen Fan wrote: my main goal is to add support migration with host NIC passthrough devices and keep the network connectivity. this series patch base on Shradha's patches on https://www.redhat.com/archives/libvir-list/2012-November/msg01324.html which is add migration support for host passthrough devices. 1) unplug the ephemeral devices before migration 2) do native migration 3) when migration finished, hotplug the ephemeral devices IMHO this algorithm is something that an upper layer management app should do. The device unplug operation is complex and it might not succeed which will make the current migration thread hang or fail in an intermediate state that will not be recoverable. However you wouldn't want each of the upper layer management apps implementing their own hacks for this; so something somewhere needs to standardise what the guest sees. The guest still will see an PCI device unplug request and will have to respond to it, then will be paused and after resume a new PCI device will appear. This is standardised. The nonstandardised part (which can't really be standardised) is how the bonding or other guest-dependant stuff will be handled, but that is up to the guest OS to handle. Why can't that be standardised? Don't we need to provide the information on what to bond to the guest and that this process is happening? The previous suggestion was to use guest-agent for this. When we've had standardized policy decisions in libvirt before it has come back to bite us later. The block migration code is a prime example. Someone decided that the standardized behaviour should be that block migrate skips readonly disks. Everything looked great for a while and then a new use case came up in OpenStack for which this standardized behaviour is no longer suitable. Now we have to abandon this standardized policy and implement what we actually want in OpenStack itself. This is the key problem with applying policy decisions inside libvirt, and thus why our focus is on providing the mechanisms on which applications should build the policies they specifically desire. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC v1 0/6] Live Migration with ephemeral host NIC devices
* Peter Krempa (pkre...@redhat.com) wrote: On Wed, May 13, 2015 at 09:40:23 +0100, Dr. David Alan Gilbert wrote: * Peter Krempa (pkre...@redhat.com) wrote: On Wed, May 13, 2015 at 09:08:39 +0100, Dr. David Alan Gilbert wrote: * Peter Krempa (pkre...@redhat.com) wrote: On Wed, May 13, 2015 at 11:36:26 +0800, Chen Fan wrote: my main goal is to add support migration with host NIC passthrough devices and keep the network connectivity. this series patch base on Shradha's patches on https://www.redhat.com/archives/libvir-list/2012-November/msg01324.html which is add migration support for host passthrough devices. 1) unplug the ephemeral devices before migration 2) do native migration 3) when migration finished, hotplug the ephemeral devices IMHO this algorithm is something that an upper layer management app should do. The device unplug operation is complex and it might not succeed which will make the current migration thread hang or fail in an intermediate state that will not be recoverable. However you wouldn't want each of the upper layer management apps implementing their own hacks for this; so something somewhere needs to standardise what the guest sees. The guest still will see an PCI device unplug request and will have to respond to it, then will be paused and after resume a new PCI device will appear. This is standardised. The nonstandardised part (which can't really be standardised) is how the bonding or other guest-dependant stuff will be handled, but that is up to the guest OS to handle. Why can't that be standardised? Don't we need to provide the information on what to bond to the guest and that this process is happening? The previous suggestion was to use guest-agent for this. Well, since only in linux you've got multiple ways to do that including legacy init scripts on various distros, the systemd-networkd thingie or how it's called or network manager, standardising this part won't be that easy. Not speaking of possible different OSes. Right - so we need to standardise on the messaging we send to the guest to tell it that we've got this bonded hotplug setup, and then the different OSs can implement what they need off using that information. From libvirt's perspective this is only something that will trigger the device unplug and plug the devices back. And there are a lot of issues here: 1) the destination of the migration might not have the desired devices This will trigger a lot of problems as we will not be able to guarantee that the devices reappear on the destination and if we'd wanted to check we'd need a new migration protocol AFAIK. But if it's using the bonding trick then that isn't fatal; it would still be able to have the bonded virtio device. 2) The guest OS might refuse to detach the PCI device (it might be stuck before PCI code is loaded) In that case the migration will be stuck forever and abort attempts will make the domain state basically undefined depending on the phase where it failed. Since we can't guarantee that the unplug of the PCI host devices will be atomic or that it will succeed we basically can't guarantee in any way in which state the VM will end up later after (a possibly failed) migration. To recover such state there are too many option that could be desired by the user that would be hard to implement in a way that would be flexible enough. I don't understand why this is any different to any other PCI device hot-unplug. It's the same, but once libvirt would be doing multiple PCI unplug requests along with the migration code, things might not go well. If you then couple this with different user expectations what should happen in various error cases it gets even more messy. Well, since we've got the bond it shouldn't get quite that bad; the error cases don't sound that bad: 1) If we can't hot-unplug then we don't migrate/cancel migration. We warn the user, if we're unlucky we're left running on the bond. 2) If we can't hot-plug at the end, then we've still got the bond in, so the guest carries on running (albeit with reduced performance). We need to flag this to the user somehow. Dave Peter -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC v1 4/6] migration: Migration support for ephemeral hostdevs
On Wed, May 13, 2015 at 11:36:30AM +0800, Chen Fan wrote: add migration support for ephemeral host devices, introduce two 'detach' and 'restore' functions to unplug/plug host devices during migration. Signed-off-by: Chen Fan chen.fan.f...@cn.fujitsu.com --- src/qemu/qemu_migration.c | 171 -- src/qemu/qemu_migration.h | 9 +++ src/qemu/qemu_process.c | 11 +++ 3 files changed, 187 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 56112f9..d5a698f 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c +void +qemuMigrationRestoreEphemeralDevices(virQEMUDriverPtr driver, + virConnectPtr conn, + virDomainObjPtr vm, + bool live) +{ +qemuDomainObjPrivatePtr priv = vm-privateData; +virDomainDeviceDefPtr dev; +int ret = -1; +size_t i; + +VIR_DEBUG(Rum domain restore ephemeral devices); + +for (i = 0; i priv-nEphemeralDevices; i++) { +dev = priv-ephemeralDevices[i]; + +switch ((virDomainDeviceType) dev-type) { +case VIR_DOMAIN_DEVICE_NET: +if (live) { +ret = qemuDomainAttachNetDevice(conn, driver, vm, +dev-data.net); +} else { +ret = virDomainNetInsert(vm-def, dev-data.net); +} + +if (!ret) +dev-data.net = NULL; +break; +case VIR_DOMAIN_DEVICE_HOSTDEV: +if (live) { +ret = qemuDomainAttachHostDevice(conn, driver, vm, + dev-data.hostdev); + } else { +ret =virDomainHostdevInsert(vm-def, dev-data.hostdev); +} This re-attach step is where we actually have far far far worse problems than with detach. This is blindly assuming that the guest on the target host can use the same hostdev that it was using on the source host. This is essentially useless in the real world. Even if the same vendor/model device is available on the target host, it is very unlikely to be available at the same bus/slot/function that it was on the source. It is quite likely neccessary to allocate a complete different NIC, or if using SRIOV allocate a different function. It is also not uncommon to have different vendor/models, so a completely different NIC may be required. It is impossible for libvirt todo anything sensible when picking the hostdev to use on the target host as it does not have anywhere near enough knowledge to make a correct decision. For example, it does not know which physical network each NIC on the target host is plugged into. Even if it knew the networks, it does not know what the I/O utilization is likel, to be able to intelligently decide between a set of possible free NICs. In any non-trivial mgmt app, the management app itself will have this knowledge and have policies around which hostdevice to assign to a guest given a particular set of circumstances. It may even decide not to assign a hostdev on the target and instead provide 2 or 3 or more emulated devices that could be used in bandwidth aggregation mode rather than failover mode. In OpenStack, the compute hosts don't even decide which NICs are given to which guests. This is down to an external schedular running on a different host(s), and the compute host just hotplugs what has already been decided elsewhere. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Ignore Vim swap files.
On Sun, 2015-05-10 at 17:28 -0400, Cole Robinson wrote: Instead of adding more bits to the .gitignore that aren't specific to the project, I'd recommend you set up a global .gitignore to ignore editor bits: https://stackoverflow.com/questions/7335420/global-git-ignore Done, thanks for the pointer :) -- Andrea Bolognani abolo...@redhat.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC v1 0/6] Live Migration with ephemeral host NIC devices
On Wed, May 13, 2015 at 09:08:39 +0100, Dr. David Alan Gilbert wrote: * Peter Krempa (pkre...@redhat.com) wrote: On Wed, May 13, 2015 at 11:36:26 +0800, Chen Fan wrote: my main goal is to add support migration with host NIC passthrough devices and keep the network connectivity. this series patch base on Shradha's patches on https://www.redhat.com/archives/libvir-list/2012-November/msg01324.html which is add migration support for host passthrough devices. 1) unplug the ephemeral devices before migration 2) do native migration 3) when migration finished, hotplug the ephemeral devices IMHO this algorithm is something that an upper layer management app should do. The device unplug operation is complex and it might not succeed which will make the current migration thread hang or fail in an intermediate state that will not be recoverable. However you wouldn't want each of the upper layer management apps implementing their own hacks for this; so something somewhere needs to standardise what the guest sees. The guest still will see an PCI device unplug request and will have to respond to it, then will be paused and after resume a new PCI device will appear. This is standardised. The nonstandardised part (which can't really be standardised) is how the bonding or other guest-dependant stuff will be handled, but that is up to the guest OS to handle. From libvirt's perspective this is only something that will trigger the device unplug and plug the devices back. And there are a lot of issues here: 1) the destination of the migration might not have the desired devices This will trigger a lot of problems as we will not be able to guarantee that the devices reappear on the destination and if we'd wanted to check we'd need a new migration protocol AFAIK. 2) The guest OS might refuse to detach the PCI device (it might be stuck before PCI code is loaded) In that case the migration will be stuck forever and abort attempts will make the domain state basically undefined depending on the phase where it failed. Since we can't guarantee that the unplug of the PCI host devices will be atomic or that it will succeed we basically can't guarantee in any way in which state the VM will end up later after (a possibly failed) migration. To recover such state there are too many option that could be desired by the user that would be hard to implement in a way that would be flexible enough. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC v1 0/6] Live Migration with ephemeral host NIC devices
On Wed, May 13, 2015 at 10:00:42AM +0100, Dr. David Alan Gilbert wrote: * Peter Krempa (pkre...@redhat.com) wrote: On Wed, May 13, 2015 at 09:40:23 +0100, Dr. David Alan Gilbert wrote: * Peter Krempa (pkre...@redhat.com) wrote: On Wed, May 13, 2015 at 09:08:39 +0100, Dr. David Alan Gilbert wrote: * Peter Krempa (pkre...@redhat.com) wrote: On Wed, May 13, 2015 at 11:36:26 +0800, Chen Fan wrote: my main goal is to add support migration with host NIC passthrough devices and keep the network connectivity. this series patch base on Shradha's patches on https://www.redhat.com/archives/libvir-list/2012-November/msg01324.html which is add migration support for host passthrough devices. 1) unplug the ephemeral devices before migration 2) do native migration 3) when migration finished, hotplug the ephemeral devices IMHO this algorithm is something that an upper layer management app should do. The device unplug operation is complex and it might not succeed which will make the current migration thread hang or fail in an intermediate state that will not be recoverable. However you wouldn't want each of the upper layer management apps implementing their own hacks for this; so something somewhere needs to standardise what the guest sees. The guest still will see an PCI device unplug request and will have to respond to it, then will be paused and after resume a new PCI device will appear. This is standardised. The nonstandardised part (which can't really be standardised) is how the bonding or other guest-dependant stuff will be handled, but that is up to the guest OS to handle. Why can't that be standardised? Don't we need to provide the information on what to bond to the guest and that this process is happening? The previous suggestion was to use guest-agent for this. Well, since only in linux you've got multiple ways to do that including legacy init scripts on various distros, the systemd-networkd thingie or how it's called or network manager, standardising this part won't be that easy. Not speaking of possible different OSes. Right - so we need to standardise on the messaging we send to the guest to tell it that we've got this bonded hotplug setup, and then the different OSs can implement what they need off using that information. From libvirt's perspective this is only something that will trigger the device unplug and plug the devices back. And there are a lot of issues here: 1) the destination of the migration might not have the desired devices This will trigger a lot of problems as we will not be able to guarantee that the devices reappear on the destination and if we'd wanted to check we'd need a new migration protocol AFAIK. But if it's using the bonding trick then that isn't fatal; it would still be able to have the bonded virtio device. 2) The guest OS might refuse to detach the PCI device (it might be stuck before PCI code is loaded) In that case the migration will be stuck forever and abort attempts will make the domain state basically undefined depending on the phase where it failed. Since we can't guarantee that the unplug of the PCI host devices will be atomic or that it will succeed we basically can't guarantee in any way in which state the VM will end up later after (a possibly failed) migration. To recover such state there are too many option that could be desired by the user that would be hard to implement in a way that would be flexible enough. I don't understand why this is any different to any other PCI device hot-unplug. It's the same, but once libvirt would be doing multiple PCI unplug requests along with the migration code, things might not go well. If you then couple this with different user expectations what should happen in various error cases it gets even more messy. Well, since we've got the bond it shouldn't get quite that bad; the error cases don't sound that bad: 1) If we can't hot-unplug then we don't migrate/cancel migration. We warn the user, if we're unlucky we're left running on the bond. 2) If we can't hot-plug at the end, then we've still got the bond in, so the guest carries on running (albeit with reduced performance). We need to flag this to the user somehow. If there are multiple PCI devices attached to the guest, we may end up with some PCI devices removed and some still present, and some for which we don't know if they are removed or present at all as the guest may simply not have responded to us yet. Further there are devices which are not just bonded NICs, so I'm really not happy for us to design a policy that works for bonded NICs but which is
Re: [libvirt] [RFC v1 4/6] migration: Migration support for ephemeral hostdevs
On Wed, May 13, 2015 at 10:36:34AM +0200, Peter Krempa wrote: On Wed, May 13, 2015 at 11:36:30 +0800, Chen Fan wrote: add migration support for ephemeral host devices, introduce two 'detach' and 'restore' functions to unplug/plug host devices during migration. Signed-off-by: Chen Fan chen.fan.f...@cn.fujitsu.com --- src/qemu/qemu_migration.c | 171 -- src/qemu/qemu_migration.h | 9 +++ src/qemu/qemu_process.c | 11 +++ 3 files changed, 187 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 56112f9..d5a698f 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3384,6 +3384,158 @@ qemuMigrationPrepareDef(virQEMUDriverPtr driver, return def; } +int +qemuMigrationDetachEphemeralDevices(virQEMUDriverPtr driver, +virDomainObjPtr vm, +bool live) +{ +qemuDomainObjPrivatePtr priv = vm-privateData; +virDomainHostdevDefPtr hostdev; +virDomainNetDefPtr net; +virDomainDeviceDef dev; +virDomainDeviceDefPtr dev_copy = NULL; +virCapsPtr caps = NULL; +int actualType; +int ret = -1; +size_t i; + +VIR_DEBUG(Rum domain detach ephemeral devices); + +if (!(caps = virQEMUDriverGetCapabilities(driver, false))) +return ret; + +for (i = 0; i vm-def-nnets;) { +net = vm-def-nets[i]; + +actualType = virDomainNetGetActualType(net); +if (actualType != VIR_DOMAIN_NET_TYPE_HOSTDEV) { +i++; +continue; +} + +hostdev = virDomainNetGetActualHostdev(net); +if (hostdev-mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS || +hostdev-source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI || +!hostdev-ephemeral) { +i++; +continue; +} + +dev.type = VIR_DOMAIN_DEVICE_NET; +dev.data.net = net; + +dev_copy = virDomainDeviceDefCopy(dev, vm-def, + caps, driver-xmlopt); +if (!dev_copy) +goto cleanup; + +if (live) { +/* nnets reduced */ +if (qemuDomainDetachNetDevice(driver, vm, dev_copy) 0) +goto cleanup; So this is where the fun begins. qemuDomainDetachNetDevice is not designed to be called this way since the detach API where it's used normally returns 0 in the following two cases: 1) The detach was successfull, the guest removed the device 2) The detach request was successful, but guest did not remove the device yet In the latter case you need to wait for a event to successfully know when the device was removed. Since this might very well happen the code will need to be changed to take that option into account. Please note that that step will make all the things really complicated. Even more fun 3) The detach request was successful, but the guest is going to ignore it forever Really, this is not something we want to be deciding policy for inside libvirt. It is no end of trouble and we really must let the mgmt app decide how it wants this kind of problem handled. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC v1 0/6] Live Migration with ephemeral host NIC devices
* Daniel P. Berrange (berra...@redhat.com) wrote: On Wed, May 13, 2015 at 10:00:42AM +0100, Dr. David Alan Gilbert wrote: * Peter Krempa (pkre...@redhat.com) wrote: On Wed, May 13, 2015 at 09:40:23 +0100, Dr. David Alan Gilbert wrote: * Peter Krempa (pkre...@redhat.com) wrote: On Wed, May 13, 2015 at 09:08:39 +0100, Dr. David Alan Gilbert wrote: * Peter Krempa (pkre...@redhat.com) wrote: On Wed, May 13, 2015 at 11:36:26 +0800, Chen Fan wrote: my main goal is to add support migration with host NIC passthrough devices and keep the network connectivity. this series patch base on Shradha's patches on https://www.redhat.com/archives/libvir-list/2012-November/msg01324.html which is add migration support for host passthrough devices. 1) unplug the ephemeral devices before migration 2) do native migration 3) when migration finished, hotplug the ephemeral devices IMHO this algorithm is something that an upper layer management app should do. The device unplug operation is complex and it might not succeed which will make the current migration thread hang or fail in an intermediate state that will not be recoverable. However you wouldn't want each of the upper layer management apps implementing their own hacks for this; so something somewhere needs to standardise what the guest sees. The guest still will see an PCI device unplug request and will have to respond to it, then will be paused and after resume a new PCI device will appear. This is standardised. The nonstandardised part (which can't really be standardised) is how the bonding or other guest-dependant stuff will be handled, but that is up to the guest OS to handle. Why can't that be standardised? Don't we need to provide the information on what to bond to the guest and that this process is happening? The previous suggestion was to use guest-agent for this. Well, since only in linux you've got multiple ways to do that including legacy init scripts on various distros, the systemd-networkd thingie or how it's called or network manager, standardising this part won't be that easy. Not speaking of possible different OSes. Right - so we need to standardise on the messaging we send to the guest to tell it that we've got this bonded hotplug setup, and then the different OSs can implement what they need off using that information. From libvirt's perspective this is only something that will trigger the device unplug and plug the devices back. And there are a lot of issues here: 1) the destination of the migration might not have the desired devices This will trigger a lot of problems as we will not be able to guarantee that the devices reappear on the destination and if we'd wanted to check we'd need a new migration protocol AFAIK. But if it's using the bonding trick then that isn't fatal; it would still be able to have the bonded virtio device. 2) The guest OS might refuse to detach the PCI device (it might be stuck before PCI code is loaded) In that case the migration will be stuck forever and abort attempts will make the domain state basically undefined depending on the phase where it failed. Since we can't guarantee that the unplug of the PCI host devices will be atomic or that it will succeed we basically can't guarantee in any way in which state the VM will end up later after (a possibly failed) migration. To recover such state there are too many option that could be desired by the user that would be hard to implement in a way that would be flexible enough. I don't understand why this is any different to any other PCI device hot-unplug. It's the same, but once libvirt would be doing multiple PCI unplug requests along with the migration code, things might not go well. If you then couple this with different user expectations what should happen in various error cases it gets even more messy. Well, since we've got the bond it shouldn't get quite that bad; the error cases don't sound that bad: 1) If we can't hot-unplug then we don't migrate/cancel migration. We warn the user, if we're unlucky we're left running on the bond. 2) If we can't hot-plug at the end, then we've still got the bond in, so the guest carries on running (albeit with reduced performance). We need to flag this to the user somehow. If there are multiple PCI devices attached to the guest, we may end up with some PCI devices removed and some still present, and some for which we don't know if they are removed or
Re: [libvirt] [PATCHv2] Ignore bridge template names with multiple printf conversions
On 05/05/2015 12:39 PM, Eric Blake wrote: On 05/05/2015 10:26 AM, Ján Tomko wrote: On Tue, May 05, 2015 at 10:14:18AM -0600, Eric Blake wrote: On 05/05/2015 10:05 AM, Ján Tomko wrote: For some reason, we allow a bridge name with %d in it, which we replace with an unsigned integer to form a bridge name that does not yet exist on the host. +if (def-bridge +(p = strchr(def-bridge, '%')) == strrchr(def-bridge, '%') +strstr(def-bridge, %d)) Simpler as: if (def-bridge strstr(def-bridge, %d) == strrchr(def-bridge, '%')) I still don't see it. [A] strchr(def-bridge, '%') [B] strrchr(def-bridge, '%') [C] strstr(def-bridge, %d)) When def-bridge is '%s%s%s%d', [A] points to the first %s, [B] points to the %d and so does [C] This string would pass the simplified condition (B == C), but not the full one (A != C) Okay, I see your counterargument. Still, strstr() is pretty expensive compared to just: if (def-bridge (p = strchr(def-bridge, '%')) == strrchr(def-bridge, '%') p[1] == 'd') Coverity complains : Event returned_null:strchr returns null (checked 273 out of 288 times). Event var_assigned: Assigning: p = null return value from strchr. Event cond_true:Condition (p = strchr(def-bridge, 37)) == strrchr(def-bridge, 37), taking true branch Event dereference: Dereferencing a null pointer p. John Jan ACK with that simplification. at which point p is no longer a dead variable, but I've still reduced the code complexity. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC v1 4/6] migration: Migration support for ephemeral hostdevs
On Wed, May 13, 2015 at 11:36:30 +0800, Chen Fan wrote: add migration support for ephemeral host devices, introduce two 'detach' and 'restore' functions to unplug/plug host devices during migration. Signed-off-by: Chen Fan chen.fan.f...@cn.fujitsu.com --- src/qemu/qemu_migration.c | 171 -- src/qemu/qemu_migration.h | 9 +++ src/qemu/qemu_process.c | 11 +++ 3 files changed, 187 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 56112f9..d5a698f 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3384,6 +3384,158 @@ qemuMigrationPrepareDef(virQEMUDriverPtr driver, return def; } +int +qemuMigrationDetachEphemeralDevices(virQEMUDriverPtr driver, +virDomainObjPtr vm, +bool live) +{ +qemuDomainObjPrivatePtr priv = vm-privateData; +virDomainHostdevDefPtr hostdev; +virDomainNetDefPtr net; +virDomainDeviceDef dev; +virDomainDeviceDefPtr dev_copy = NULL; +virCapsPtr caps = NULL; +int actualType; +int ret = -1; +size_t i; + +VIR_DEBUG(Rum domain detach ephemeral devices); + +if (!(caps = virQEMUDriverGetCapabilities(driver, false))) +return ret; + +for (i = 0; i vm-def-nnets;) { +net = vm-def-nets[i]; + +actualType = virDomainNetGetActualType(net); +if (actualType != VIR_DOMAIN_NET_TYPE_HOSTDEV) { +i++; +continue; +} + +hostdev = virDomainNetGetActualHostdev(net); +if (hostdev-mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS || +hostdev-source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI || +!hostdev-ephemeral) { +i++; +continue; +} + +dev.type = VIR_DOMAIN_DEVICE_NET; +dev.data.net = net; + +dev_copy = virDomainDeviceDefCopy(dev, vm-def, + caps, driver-xmlopt); +if (!dev_copy) +goto cleanup; + +if (live) { +/* nnets reduced */ +if (qemuDomainDetachNetDevice(driver, vm, dev_copy) 0) +goto cleanup; So this is where the fun begins. qemuDomainDetachNetDevice is not designed to be called this way since the detach API where it's used normally returns 0 in the following two cases: 1) The detach was successfull, the guest removed the device 2) The detach request was successful, but guest did not remove the device yet In the latter case you need to wait for a event to successfully know when the device was removed. Since this might very well happen the code will need to be changed to take that option into account. Please note that that step will make all the things really complicated. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC v1 0/6] Live Migration with ephemeral host NIC devices
On Wed, May 13, 2015 at 09:40:23 +0100, Dr. David Alan Gilbert wrote: * Peter Krempa (pkre...@redhat.com) wrote: On Wed, May 13, 2015 at 09:08:39 +0100, Dr. David Alan Gilbert wrote: * Peter Krempa (pkre...@redhat.com) wrote: On Wed, May 13, 2015 at 11:36:26 +0800, Chen Fan wrote: my main goal is to add support migration with host NIC passthrough devices and keep the network connectivity. this series patch base on Shradha's patches on https://www.redhat.com/archives/libvir-list/2012-November/msg01324.html which is add migration support for host passthrough devices. 1) unplug the ephemeral devices before migration 2) do native migration 3) when migration finished, hotplug the ephemeral devices IMHO this algorithm is something that an upper layer management app should do. The device unplug operation is complex and it might not succeed which will make the current migration thread hang or fail in an intermediate state that will not be recoverable. However you wouldn't want each of the upper layer management apps implementing their own hacks for this; so something somewhere needs to standardise what the guest sees. The guest still will see an PCI device unplug request and will have to respond to it, then will be paused and after resume a new PCI device will appear. This is standardised. The nonstandardised part (which can't really be standardised) is how the bonding or other guest-dependant stuff will be handled, but that is up to the guest OS to handle. Why can't that be standardised? Don't we need to provide the information on what to bond to the guest and that this process is happening? The previous suggestion was to use guest-agent for this. Well, since only in linux you've got multiple ways to do that including legacy init scripts on various distros, the systemd-networkd thingie or how it's called or network manager, standardising this part won't be that easy. Not speaking of possible different OSes. From libvirt's perspective this is only something that will trigger the device unplug and plug the devices back. And there are a lot of issues here: 1) the destination of the migration might not have the desired devices This will trigger a lot of problems as we will not be able to guarantee that the devices reappear on the destination and if we'd wanted to check we'd need a new migration protocol AFAIK. But if it's using the bonding trick then that isn't fatal; it would still be able to have the bonded virtio device. 2) The guest OS might refuse to detach the PCI device (it might be stuck before PCI code is loaded) In that case the migration will be stuck forever and abort attempts will make the domain state basically undefined depending on the phase where it failed. Since we can't guarantee that the unplug of the PCI host devices will be atomic or that it will succeed we basically can't guarantee in any way in which state the VM will end up later after (a possibly failed) migration. To recover such state there are too many option that could be desired by the user that would be hard to implement in a way that would be flexible enough. I don't understand why this is any different to any other PCI device hot-unplug. It's the same, but once libvirt would be doing multiple PCI unplug requests along with the migration code, things might not go well. If you then couple this with different user expectations what should happen in various error cases it gets even more messy. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] sysinfo: Fix reports on ARM
On Wed, May 13, 2015 at 01:55:43PM +0200, Michal Privoznik wrote: Due to a kernel commit (b4b8f770e), cpuinfo format has changed on ARMs. Firstly, 'Processor: ...' may not be reported, it's replaced by 'model name: ...'. Secondly, the Processor string may occur in CPU name, e.g. 'ARMv7 Processor rev 5 (v7l)'. Therefore, we must firstly look for 'model name' and then for 'Processor' if not found. Moreover, lines in the cpuinfo file are shuffled, so we better not manipulate the pointer to start of internal buffer as we may lost some info. Signed-off-by: Michal Privoznik mpriv...@redhat.com [snip] diff --git a/tests/sysinfodata/arm-rpi2cpuinfo.data b/tests/sysinfodata/arm-rpi2cpuinfo.data new file mode 100644 index 000..41fde00 --- /dev/null +++ b/tests/sysinfodata/arm-rpi2cpuinfo.data @@ -0,0 +1,43 @@ +processor : 0 +model name : ARMv7 Processor rev 5 (v7l) +BogoMIPS: 38.40 +Features: half thumb fastmult vfp edsp neon vfpv3 tls vfpv4 idiva idivt vfpd32 lpae evtstrm +CPU implementer : 0x41 +CPU architecture: 7 +CPU variant : 0x0 +CPU part: 0xc07 +CPU revision: 5 It occurs to me that our code would clearer and more robust if we changed the way we dealt with the cpu info files from proc. Instead of doing all the insane strchr/strstr searches and pointer manipulation we could usefully have a generic parsing method that does: - Read file contents - Split file into lines - For each line - Split on ':' - Strip whitespace from each part - Store left hand side as key, right hand side as value in a virTypedParameter - Returns an array of arrays of virTypedParameter Then, the code for extract information from the file merely has to iterate over the outer array to get each processor. For each processor it can then do exact matches on the key name. This avoids the kind of issue where we're matching on 'processor' but that can occurr in the value too. I've no objection to your proposed patch though. This is just a thought for someone motivated enough to cleanup this area of code. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 3/5] Move QEMU-only fields from virDomainDiskDef into privateData
Signed-off-by: Jiri Denemark jdene...@redhat.com --- Notes: Version 2: - new patch src/conf/domain_conf.c| 6 - src/conf/domain_conf.h| 10 src/libvirt_private.syms | 1 - src/qemu/qemu_blockjob.c | 48 + src/qemu/qemu_domain.c| 61 +++ src/qemu/qemu_domain.h| 21 src/qemu/qemu_driver.c| 11 + src/qemu/qemu_migration.c | 6 +++-- src/qemu/qemu_process.c | 17 +++-- 9 files changed, 125 insertions(+), 56 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3204140..bf0099d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1290,11 +1290,6 @@ virDomainDiskDefNew(virDomainXMLOptionPtr xmlopt) !(ret-privateData = xmlopt-privateData.diskNew())) goto error; -if (virCondInit(ret-blockJobSyncCond) 0) { -virReportSystemError(errno, %s, _(Failed to initialize condition)); -goto error; -} - return ret; error: @@ -1319,7 +1314,6 @@ virDomainDiskDefFree(virDomainDiskDefPtr def) VIR_FREE(def-domain_name); virDomainDeviceInfoClear(def-info); virObjectUnref(def-privateData); -virCondDestroy(def-blockJobSyncCond); VIR_FREE(def); } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index b5e7617..8312c20 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -691,20 +691,10 @@ struct _virDomainDiskDef { int tray_status; /* enum virDomainDiskTray */ int removable; /* enum virTristateSwitch */ -/* ideally we want a smarter way to interlock block jobs on single qemu disk - * in the future, but for now we just disallow any concurrent job on a - * single disk */ -bool blockjob; virStorageSourcePtr mirror; int mirrorState; /* enum virDomainDiskMirrorState */ int mirrorJob; /* virDomainBlockJobType */ -/* for some synchronous block jobs, we need to notify the owner */ -virCond blockJobSyncCond; -int blockJobType; /* type of the block job from the event */ -int blockJobStatus; /* status of the finished block job */ -bool blockJobSync; /* the block job needs synchronized termination */ - struct { unsigned int cylinders; unsigned int heads; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 67a7e21..2586572 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -308,7 +308,6 @@ virDomainGraphicsTypeFromString; virDomainGraphicsTypeToString; virDomainGraphicsVNCSharePolicyTypeFromString; virDomainGraphicsVNCSharePolicyTypeToString; -virDomainHasBlockjob; virDomainHasNet; virDomainHostdevCapsTypeToString; virDomainHostdevDefAlloc; diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c index 729928a..b9572d0 100644 --- a/src/qemu/qemu_blockjob.c +++ b/src/qemu/qemu_blockjob.c @@ -65,6 +65,7 @@ qemuBlockJobEventProcess(virQEMUDriverPtr driver, virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); virDomainDiskDefPtr persistDisk = NULL; bool save = false; +qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); /* Have to generate two variants of the event for old vs. new * client callbacks */ @@ -127,7 +128,7 @@ qemuBlockJobEventProcess(virQEMUDriverPtr driver, disk-mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN; ignore_value(qemuDomainDetermineDiskChain(driver, vm, disk, true, true)); -disk-blockjob = false; +diskPriv-blockjob = false; break; case VIR_DOMAIN_BLOCK_JOB_READY: @@ -143,7 +144,7 @@ qemuBlockJobEventProcess(virQEMUDriverPtr driver, VIR_DOMAIN_DISK_MIRROR_STATE_ABORT : VIR_DOMAIN_DISK_MIRROR_STATE_NONE; disk-mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN; save = true; -disk-blockjob = false; +diskPriv-blockjob = false; break; case VIR_DOMAIN_BLOCK_JOB_LAST: @@ -185,11 +186,13 @@ qemuBlockJobEventProcess(virQEMUDriverPtr driver, void qemuBlockJobSyncBegin(virDomainDiskDefPtr disk) { -if (disk-blockJobSync) +qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); + +if (diskPriv-blockJobSync) VIR_WARN(Disk %s already has synchronous block job, disk-dst); -disk-blockJobSync = true; +diskPriv-blockJobSync = true; } @@ -211,15 +214,17 @@ qemuBlockJobSyncEnd(virQEMUDriverPtr driver, virDomainDiskDefPtr disk, virConnectDomainEventBlockJobStatus *ret_status) { -if (disk-blockJobSync disk-blockJobStatus != -1) { +qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); + +if (diskPriv-blockJobSync diskPriv-blockJobStatus != -1) { if (ret_status) -*ret_status = disk-blockJobStatus; +*ret_status =
Re: [libvirt] [PATCH] parallels: remove connection wide wait timeout
On 05/07/2015 03:20 PM, Nikolay Shirokovskiy wrote: We have a lot of passing arguments code just to pass connection object cause it holds jobTimeout. Taking into account that right now this value is defined at compile time let's just get rid of it and make arguments list more clear in many places. In case we later need some runtime configurable timeout value we can provide this value through arguments function already operate such as a parallels domain object etc as this timeouts are operation( and thus object) specific in practice. I agree with this patch, at this point hardcoded timeout value has no sense. But could you, please, rebase the patch and fix parallelsDomainDetachDeviceFlags? Signed-off-by: Nikolay Shirokovskiy nshirokovs...@parallels.com --- src/parallels/parallels_driver.c |6 +- src/parallels/parallels_sdk.c| 98 +- src/parallels/parallels_sdk.h| 22 - src/parallels/parallels_utils.h |1 - 4 files changed, 57 insertions(+), 70 deletions(-) diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c index 3aa87ca..63bf638 100644 --- a/src/parallels/parallels_driver.c +++ b/src/parallels/parallels_driver.c @@ -214,7 +214,7 @@ parallelsOpenDefault(virConnectPtr conn) goto err_free; } -if (prlsdkInit(privconn)) { +if (prlsdkInit()) { VIR_DEBUG(%s, _(Can't initialize Parallels SDK)); goto err_free; } @@ -1087,7 +1087,7 @@ parallelsDomainManagedSaveRemove(virDomainPtr domain, unsigned int flags) if (!(state == VIR_DOMAIN_SHUTOFF reason == VIR_DOMAIN_SHUTOFF_SAVED)) goto cleanup; -ret = prlsdkDomainManagedSaveRemove(privconn, dom); +ret = prlsdkDomainManagedSaveRemove(dom); cleanup: virObjectUnlock(dom); @@ -1139,7 +1139,7 @@ static int parallelsDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, switch (dev-type) { case VIR_DOMAIN_DEVICE_DISK: -ret = prlsdkAttachVolume(dom-conn, privdom, dev-data.disk); +ret = prlsdkAttachVolume(privdom, dev-data.disk); if (ret) { virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(disk attach failed)); diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c index e1ba7cf..83de1de 100644 --- a/src/parallels/parallels_sdk.c +++ b/src/parallels/parallels_sdk.c @@ -35,8 +35,6 @@ #define VIR_FROM_THIS VIR_FROM_PARALLELS #define JOB_INFINIT_WAIT_TIMEOUT UINT_MAX -PRL_UINT32 defaultJobTimeout = JOB_INFINIT_WAIT_TIMEOUT; - VIR_LOG_INIT(parallels.sdk); /* @@ -179,9 +177,9 @@ getJobResultHelper(PRL_HANDLE job, unsigned int timeout, PRL_HANDLE *result, return ret; } -#define getJobResult(job, timeout, result) \ -getJobResultHelper(job, timeout, result, __FILE__, \ - __FUNCTION__, __LINE__) +#define getJobResult(job, result) \ +getJobResultHelper(job, JOB_INFINIT_WAIT_TIMEOUT, \ +result, __FILE__, __FUNCTION__, __LINE__) static PRL_RESULT waitJobHelper(PRL_HANDLE job, unsigned int timeout, @@ -197,12 +195,13 @@ waitJobHelper(PRL_HANDLE job, unsigned int timeout, return ret; } -#define waitJob(job, timeout) \ -waitJobHelper(job, timeout, __FILE__, \ +#define waitJob(job)\ +waitJobHelper(job, JOB_INFINIT_WAIT_TIMEOUT, __FILE__, \ __FUNCTION__, __LINE__) + int -prlsdkInit(parallelsConnPtr privconn) +prlsdkInit(void) { PRL_RESULT ret; @@ -212,8 +211,6 @@ prlsdkInit(parallelsConnPtr privconn) return -1; } -privconn-jobTimeout = JOB_INFINIT_WAIT_TIMEOUT; - return 0; }; @@ -238,7 +235,7 @@ prlsdkConnect(parallelsConnPtr privconn) job = PrlSrv_LoginLocalEx(privconn-server, NULL, 0, PSL_HIGH_SECURITY, PACF_NON_INTERACTIVE_MODE); -if (waitJob(job, privconn-jobTimeout)) { +if (waitJob(job)) { PrlHandle_Free(privconn-server); return -1; } @@ -252,7 +249,7 @@ prlsdkDisconnect(parallelsConnPtr privconn) PRL_HANDLE job; job = PrlSrv_Logoff(privconn-server); -waitJob(job, privconn-jobTimeout); +waitJob(job); PrlHandle_Free(privconn-server); } @@ -269,7 +266,7 @@ prlsdkSdkDomainLookup(parallelsConnPtr privconn, int ret = -1; job = PrlSrv_GetVmConfig(privconn-server, id, flags); -if (PRL_FAILED(getJobResult(job, privconn-jobTimeout, result))) +if (PRL_FAILED(getJobResult(job, result))) goto cleanup; pret = PrlResult_GetParamByIndex(result, 0, sdkdom); @@ -374,9 +371,7 @@ prlsdkGetDomainIds(PRL_HANDLE sdkdom, } static int -prlsdkGetDomainState(parallelsConnPtr privconn, - PRL_HANDLE sdkdom, -
[libvirt] [PATCH 4/4] Some alignment fixes in lxc_controller and jsontest
Again, a clean-up for which we don't have proper syntax-check. Signed-off-by: Martin Kletzander mklet...@redhat.com --- src/lxc/lxc_controller.c | 16 tests/jsontest.c | 12 ++-- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 4544cd0..ba44e09 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -454,7 +454,7 @@ static int virLXCControllerSetupLoopDeviceDisk(virDomainDiskDefPtr disk) cleanup: VIR_FREE(loname); if (ret 0) - VIR_FORCE_CLOSE(lofd); +VIR_FORCE_CLOSE(lofd); return lofd; @@ -1126,7 +1126,7 @@ static void virLXCControllerConsoleIO(int watch, int fd, int events, void *opaqu len = console-fromContLen; avail = sizeof(console-fromContBuf) - *len; } -reread: + reread: done = read(fd, buf + *len, avail); if (done == -1 errno == EINTR) goto reread; @@ -1154,7 +1154,7 @@ static void virLXCControllerConsoleIO(int watch, int fd, int events, void *opaqu len = console-fromHostLen; } -rewrite: + rewrite: done = write(fd, buf, *len); if (done == -1 errno == EINTR) goto rewrite; @@ -1895,7 +1895,7 @@ static int virLXCControllerMoveInterfaces(virLXCControllerPtr ctrl) virDomainHostdevCaps hdcaps = hdev-source.caps; if (hdcaps.type != VIR_DOMAIN_HOSTDEV_CAPS_TYPE_NET) - continue; +continue; if (virNetDevSetNamespace(hdcaps.u.net.iface, ctrl-initpid) 0) return -1; @@ -1985,7 +1985,7 @@ lxcCreateTty(virLXCControllerPtr ctrl, int *ttymaster, * anything other than 0, but let's play it safe. */ if ((virAsprintf(ttyName, /dev/pts/%d, ptyno) 0) || (virAsprintf(ttyHostPath, /%s/%s.devpts/%d, LXC_STATE_DIR, -ctrl-def-name, ptyno) 0)) { + ctrl-def-name, ptyno) 0)) { errno = ENOMEM; goto cleanup; } @@ -2101,7 +2101,7 @@ virLXCControllerSetupDevPTS(virLXCControllerPtr ctrl) if ((lxcContainerChown(ctrl-def, ctrl-devptmx) 0) || (lxcContainerChown(ctrl-def, devpts) 0)) - goto cleanup; +goto cleanup; ret = 0; @@ -2139,7 +2139,7 @@ virLXCControllerSetupConsoles(virLXCControllerPtr ctrl, ctrl-consoles[i].contFd, containerTTYPaths[i], ttyHostPath) 0) { virReportSystemError(errno, %s, - _(Failed to allocate tty)); + _(Failed to allocate tty)); goto cleanup; } @@ -2429,7 +2429,7 @@ int main(int argc, char *argv[]) int c; c = getopt_long(argc, argv, dn:v:p:m:c:s:h:S:, - options, NULL); +options, NULL); if (c == -1) break; diff --git a/tests/jsontest.c b/tests/jsontest.c index f27943e..f070649 100644 --- a/tests/jsontest.c +++ b/tests/jsontest.c @@ -71,7 +71,7 @@ testJSONAddRemove(const void *data) case 1: if (!info-pass) { VIR_TEST_VERBOSE(should not remove from non-object %s\n, -info-doc); + info-doc); goto cleanup; } break; @@ -83,17 +83,17 @@ testJSONAddRemove(const void *data) goto cleanup; default: VIR_TEST_VERBOSE(unexpected result when removing from %s\n, -info-doc); + info-doc); goto cleanup; } if (STRNEQ_NULLABLE(virJSONValueGetString(name), sample)) { VIR_TEST_VERBOSE(unexpected value after removing name: %s\n, -NULLSTR(virJSONValueGetString(name))); + NULLSTR(virJSONValueGetString(name))); goto cleanup; } if (virJSONValueObjectRemoveKey(json, name, NULL)) { VIR_TEST_VERBOSE(%s, -unexpected success when removing missing key\n); + unexpected success when removing missing key\n); goto cleanup; } if (virJSONValueObjectAppendString(json, newname, foo) 0) { @@ -139,8 +139,8 @@ mymain(void) DO_TEST_PARSE(Simple, {\return\: {}, \id\: \libvirt-1\}); DO_TEST_PARSE(NotSoSimple, {\QMP\: {\version\: {\qemu\: -{\micro\: 91, \minor\: 13, \major\: 0}, -\package\: \ (qemu-kvm-devel)\}, \capabilities\: []}}); + {\micro\: 91, \minor\: 13, \major\: 0}, + \package\: \ (qemu-kvm-devel)\}, \capabilities\: []}}); DO_TEST_PARSE(Harder, {\return\: [{\filename\: -- 2.4.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/4] gendispatch: Don't generate long lines
We don't allow it in normal code, why would it need to be in the generated one. IT also splits the line in perl code so it's readable. Signed-off-by: Martin Kletzander mklet...@redhat.com --- src/rpc/gendispatch.pl | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl index b642d6e..cb8e157 100755 --- a/src/rpc/gendispatch.pl +++ b/src/rpc/gendispatch.pl @@ -1,6 +1,6 @@ #!/usr/bin/perl -w # -# Copyright (C) 2010-2014 Red Hat, Inc. +# Copyright (C) 2010-2015 Red Hat, Inc. # # This library is free software; you can redistribute it and/or # modify it under the terms of the GNU Lesser General Public @@ -429,7 +429,8 @@ elsif ($mode eq server) { print {\n; print int rv;\n; print virThreadJobSet(\$name\);\n; -print VIR_DEBUG(\server=%p client=%p msg=%p rerr=%p args=%p ret=%p\, server, client, msg, rerr, args, ret);\n; +print VIR_DEBUG(\server=%p client=%p msg=%p rerr=%p args=%p ret=%p\,\n; +print server, client, msg, rerr, args, ret);\n; print rv = $name(server, client, msg, rerr; if ($argtype ne void) { print , args; -- 2.4.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/4] virnetserver: Remove unnecessary double space
Since we don't have syntax-check for this, it has to be checked manually. Let's hope this is the only place it happened. Signed-off-by: Martin Kletzander mklet...@redhat.com --- src/rpc/virnetserver.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index b061275..42427dc 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -499,7 +499,7 @@ virNetServerPtr virNetServerNewPostExecRestart(virJSONValuePtr object, goto error; } -n = virJSONValueArraySize(services); +n = virJSONValueArraySize(services); if (n 0) { virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(Malformed services data in JSON document)); @@ -532,7 +532,7 @@ virNetServerPtr virNetServerNewPostExecRestart(virJSONValuePtr object, goto error; } -n = virJSONValueArraySize(clients); +n = virJSONValueArraySize(clients); if (n 0) { virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(Malformed clients data in JSON document)); -- 2.4.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 4/5] qemu: Keep track of what disks are being migrated
Instead of redoing the same filtering over and over everytime we need to walk through all disks which are being migrated. Signed-off-by: Jiri Denemark jdene...@redhat.com --- Notes: Version 2: - make use of per-disk private data src/qemu/qemu_domain.h| 2 ++ src/qemu/qemu_migration.c | 25 - 2 files changed, 10 insertions(+), 17 deletions(-) diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 53df1d3..a6df199 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -218,6 +218,8 @@ struct _qemuDomainDiskPrivate { int blockJobType; /* type of the block job from the event */ int blockJobStatus; /* status of the finished block job */ bool blockJobSync; /* the block job needs synchronized termination */ + +bool migrating; /* the disk is being migrated */ }; typedef enum { diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 7472b09..c4f1c48 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1745,13 +1745,7 @@ qemuMigrationCheckDriveMirror(virQEMUDriverPtr driver, virDomainDiskDefPtr disk = vm-def-disks[i]; qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); -/* skip shared, RO and source-less disks */ -if (disk-src-shared || disk-src-readonly || -!virDomainDiskGetSource(disk)) -continue; - -/* skip disks that didn't start mirroring */ -if (!diskPriv-blockJobSync) +if (!diskPriv-migrating || !diskPriv-blockJobSync) continue; /* process any pending event */ @@ -1874,17 +1868,13 @@ qemuMigrationCancelDriveMirror(virQEMUDriverPtr driver, virDomainDiskDefPtr disk = vm-def-disks[i]; qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); -/* skip shared, RO and source-less disks */ -if (disk-src-shared || disk-src-readonly || -!virDomainDiskGetSource(disk)) -continue; - -/* skip disks that didn't start mirroring */ -if (!diskPriv-blockJobSync) +if (!diskPriv-migrating || !diskPriv-blockJobSync) continue; if (qemuMigrationCancelOneDriveMirror(driver, vm, disk) 0) return -1; + +diskPriv-migrating = false; } return 0; @@ -1945,6 +1935,7 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver, for (i = 0; i vm-def-ndisks; i++) { virDomainDiskDefPtr disk = vm-def-disks[i]; +qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); int mon_ret; /* skip shared, RO and source-less disks */ @@ -1975,16 +1966,16 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver, qemuBlockJobSyncEnd(driver, vm, disk, NULL); goto cleanup; } +diskPriv-migrating = true; } /* Wait for each disk to become ready in turn, but check the status * for *all* mirrors to determine if any have aborted. */ for (i = 0; i vm-def-ndisks; i++) { virDomainDiskDefPtr disk = vm-def-disks[i]; +qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); -/* skip shared, RO and source-less disks */ -if (disk-src-shared || disk-src-readonly || -!virDomainDiskGetSource(disk)) +if (!diskPriv-migrating) continue; while (disk-mirrorState != VIR_DOMAIN_DISK_MIRROR_STATE_READY) { -- 2.4.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/4] rpc: Don't mix max_clients and max_workers in PostExecRestart
This only affected the servers that re-exec themselves, which is only virtlockd and it didn't do any mess, so this is mostly a clenaup. Signed-off-by: Martin Kletzander mklet...@redhat.com --- src/rpc/virnetserver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index 47d83ba..b061275 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -484,7 +484,7 @@ virNetServerPtr virNetServerNewPostExecRestart(virJSONValuePtr object, goto error; } -if (!(srv = virNetServerNew(min_workers, max_clients, +if (!(srv = virNetServerNew(min_workers, max_workers, priority_workers, max_clients, max_anonymous_clients, keepaliveInterval, keepaliveCount, -- 2.4.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix build --without-network
On 27.04.2015 15:49, Martin Kletzander wrote: In order not to bring in any link dependencies, bridge driver doesn't use the usual stubs as other conditionally-built code does. However, having the function as a macro imposes a problem with possibly unused variables if just defined as 0. This was worked around by using (dom=dom, iface=iface, 0) which should act like a 0 if used in a condition. However, gcc still bugs about that, so I came up with another way how to fix that. Using static inline functions in the header won't collide with anything, it fixes the bug and does one thing that the macro didn't do. It checks whenther passed variables are pointers of compatible type. It has only one downside, and that is that we need to either a) define it with ATTRIBUTE_UNUSED, which needs an exception in cfg.mk or b) do something like ignore_value(variable); in the function body. I went with the first variant. Signed-off-by: Martin Kletzander mklet...@redhat.com --- Notes: I can go with the version (b) if that's the preferred one. cfg.mk | 4 ++-- src/network/bridge_driver.h | 19 --- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/cfg.mk b/cfg.mk index 9ba2134..796ed80 100644 --- a/cfg.mk +++ b/cfg.mk @@ -1,5 +1,5 @@ # Customize Makefile.maint. -*- makefile -*- -# Copyright (C) 2008-2014 Red Hat, Inc. +# Copyright (C) 2008-2015 Red Hat, Inc. # Copyright (C) 2003-2008 Free Software Foundation, Inc. # This program is free software: you can redistribute it and/or modify @@ -1184,7 +1184,7 @@ exclude_file_name_regexp--sc_prohibit_getenv = \ ^tests/.*\.[ch]$$ exclude_file_name_regexp--sc_avoid_attribute_unused_in_header = \ - ^src/util/virlog\.h$$ + ^(src/util/virlog\.h|src/network/bridge_driver\.h)$$ exclude_file_name_regexp--sc_prohibit_mixed_case_abbreviations = \ ^src/(vbox/vbox_CAPI.*.h|esx/esx_vi.(c|h)|esx/esx_storage_backend_iscsi.c)$$ diff --git a/src/network/bridge_driver.h b/src/network/bridge_driver.h index 2f801ee..513ccf7 100644 --- a/src/network/bridge_driver.h +++ b/src/network/bridge_driver.h @@ -1,7 +1,7 @@ /* * bridge_driver.h: core driver methods for managing networks * - * Copyright (C) 2006-2013 Red Hat, Inc. + * Copyright (C) 2006-2015 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -55,11 +55,24 @@ int networkDnsmasqConfContents(virNetworkObjPtr network, # else /* Define no-op replacements that don't drag in any link dependencies. */ # define networkAllocateActualDevice(dom, iface) 0 -# define networkNotifyActualDevice(dom, iface) (dom=dom, iface=iface, 0) -# define networkReleaseActualDevice(dom, iface) (dom=dom, iface=iface, 0) # define networkGetNetworkAddress(netname, netaddr) (-2) # define networkDnsmasqConfContents(network, pidfile, configstr, \ dctx, caps) 0 + +static inline int +networkNotifyActualDevice(virDomainDefPtr dom ATTRIBUTE_UNUSED, + virDomainNetDefPtr iface ATTRIBUTE_UNUSED) +{ +return 0; +} + +static inline int +networkReleaseActualDevice(virDomainDefPtr dom ATTRIBUTE_UNUSED, + virDomainNetDefPtr iface ATTRIBUTE_UNUSED) +{ +return 0; +} + # endif typedef char *(*networkDnsmasqLeaseFileNameFunc)(const char *netname); ACK Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Taint domains using cdrom-passthrough
On 05/13/2015 03:37 AM, Peter Krempa wrote: On Tue, May 12, 2015 at 16:03:33 -0400, John Ferlan wrote: https://bugzilla.redhat.com/show_bug.cgi?id=976387 For a domain configured using the host cdrom, we should taint the domain due to problems encountered when the host and guest try to control the tray. Signed-off-by: John Ferlan jfer...@redhat.com --- src/conf/domain_conf.c | 3 ++- src/conf/domain_conf.h | 1 + src/qemu/qemu_domain.c | 6 ++ 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index add857c..a67e200 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -101,7 +101,8 @@ VIR_ENUM_IMPL(virDomainTaint, VIR_DOMAIN_TAINT_LAST, disk-probing, external-launch, host-cpu, - hook-script); + hook-script, + cdrom-passthrough); VIR_ENUM_IMPL(virDomainVirt, VIR_DOMAIN_VIRT_LAST, qemu, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 2cd105a7..0867e8b 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2280,6 +2280,7 @@ typedef enum { VIR_DOMAIN_TAINT_EXTERNAL_LAUNCH, /* Externally launched guest domain */ VIR_DOMAIN_TAINT_HOST_CPU, /* Host CPU passthrough in use */ VIR_DOMAIN_TAINT_HOOK, /* Domain (possibly) changed via hook script */ +VIR_DOMAIN_TAINT_CDROM_PASSTHROUGH,/* CDROM passthrough */ VIR_DOMAIN_TAINT_LAST } virDomainTaintFlags; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index fa8229f..b66ee89 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2031,6 +2031,12 @@ void qemuDomainObjCheckDiskTaint(virQEMUDriverPtr driver, qemuDomainObjTaint(driver, obj, VIR_DOMAIN_TAINT_HIGH_PRIVILEGES, logFD); +if (disk-device == VIR_DOMAIN_DISK_DEVICE_CDROM +virStorageSourceGetActualType(disk-src) == VIR_STORAGE_TYPE_BLOCK +disk-src-path) +qemuDomainObjTaint(driver, obj, VIR_DOMAIN_TAINT_CDROM_PASSTHROUGH, + logFD); + This won't be enough currently since you can change the media in the CDROM so that it becomes a passthrough device later in it's lifecycle. You'll need to call qemuDomainObjCheckDiskTaint in qemuDomainUpdateDeviceLive too once you'll be using it to mark those. hmm.. OK - should the similar call/check be made for NET as well in a followup - even though qemuDomainObjCheckNetTaint is primarily if a net-script exists and qemuDomainChangeNet would fail if the -script changed - if some other check is made in NetTaint in the future, then we won't miss it. I will add/squash the following into the patch (same as call in qemuDomainAttachDeviceLive) : diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f922a28..a3c964f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8200,6 +8200,7 @@ qemuDomainUpdateDeviceLive(virConnectPtr conn, switch ((virDomainDeviceType) dev-type) { case VIR_DOMAIN_DEVICE_DISK: +qemuDomainObjCheckDiskTaint(driver, vm, dev-data.disk, -1); ret = qemuDomainChangeDiskMediaLive(conn, vm, dev, driver, force); break; case VIR_DOMAIN_DEVICE_GRAPHICS: John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] sysinfo: Fix reports on ARM
Due to a kernel commit (b4b8f770e), cpuinfo format has changed on ARMs. Firstly, 'Processor: ...' may not be reported, it's replaced by 'model name: ...'. Secondly, the Processor string may occur in CPU name, e.g. 'ARMv7 Processor rev 5 (v7l)'. Therefore, we must firstly look for 'model name' and then for 'Processor' if not found. Moreover, lines in the cpuinfo file are shuffled, so we better not manipulate the pointer to start of internal buffer as we may lost some info. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- diff to v1: - Jan's comments worked in - added new test case (yes, there really is a space at EOL in cpuinfo on my ARM) src/util/virsysinfo.c| 9 +++ tests/sysinfodata/arm-rpi2cpuinfo.data | 43 tests/sysinfodata/arm-rpi2sysinfo.expect | 18 + tests/sysinfotest.c | 22 4 files changed, 82 insertions(+), 10 deletions(-) create mode 100644 tests/sysinfodata/arm-rpi2cpuinfo.data create mode 100644 tests/sysinfodata/arm-rpi2sysinfo.expect diff --git a/src/util/virsysinfo.c b/src/util/virsysinfo.c index 8bb17f0..40390ab 100644 --- a/src/util/virsysinfo.c +++ b/src/util/virsysinfo.c @@ -289,16 +289,15 @@ virSysinfoParseProcessor(const char *base, virSysinfoDefPtr ret) virSysinfoProcessorDefPtr processor; char *processor_type = NULL; -if (!(tmp_base = strstr(base, Processor))) +if (!(tmp_base = strstr(base, model name)) +!(tmp_base = strstr(base, Processor))) return 0; -base = tmp_base; -eol = strchr(base, '\n'); -cur = strchr(base, ':') + 1; +eol = strchr(tmp_base, '\n'); +cur = strchr(tmp_base, ':') + 1; virSkipSpaces(cur); if (eol VIR_STRNDUP(processor_type, cur, eol - cur) 0) goto error; -base = cur; while ((tmp_base = strstr(base, processor)) != NULL) { base = tmp_base; diff --git a/tests/sysinfodata/arm-rpi2cpuinfo.data b/tests/sysinfodata/arm-rpi2cpuinfo.data new file mode 100644 index 000..41fde00 --- /dev/null +++ b/tests/sysinfodata/arm-rpi2cpuinfo.data @@ -0,0 +1,43 @@ +processor : 0 +model name : ARMv7 Processor rev 5 (v7l) +BogoMIPS: 38.40 +Features: half thumb fastmult vfp edsp neon vfpv3 tls vfpv4 idiva idivt vfpd32 lpae evtstrm +CPU implementer : 0x41 +CPU architecture: 7 +CPU variant : 0x0 +CPU part: 0xc07 +CPU revision: 5 + +processor : 1 +model name : ARMv7 Processor rev 5 (v7l) +BogoMIPS: 38.40 +Features: half thumb fastmult vfp edsp neon vfpv3 tls vfpv4 idiva idivt vfpd32 lpae evtstrm +CPU implementer : 0x41 +CPU architecture: 7 +CPU variant : 0x0 +CPU part: 0xc07 +CPU revision: 5 + +processor : 2 +model name : ARMv7 Processor rev 5 (v7l) +BogoMIPS: 38.40 +Features: half thumb fastmult vfp edsp neon vfpv3 tls vfpv4 idiva idivt vfpd32 lpae evtstrm +CPU implementer : 0x41 +CPU architecture: 7 +CPU variant : 0x0 +CPU part: 0xc07 +CPU revision: 5 + +processor : 3 +model name : ARMv7 Processor rev 5 (v7l) +BogoMIPS: 38.40 +Features: half thumb fastmult vfp edsp neon vfpv3 tls vfpv4 idiva idivt vfpd32 lpae evtstrm +CPU implementer : 0x41 +CPU architecture: 7 +CPU variant : 0x0 +CPU part: 0xc07 +CPU revision: 5 + +Hardware: BCM2709 +Revision: a01041 +Serial : c9e9323d diff --git a/tests/sysinfodata/arm-rpi2sysinfo.expect b/tests/sysinfodata/arm-rpi2sysinfo.expect new file mode 100644 index 000..acb3ad9 --- /dev/null +++ b/tests/sysinfodata/arm-rpi2sysinfo.expect @@ -0,0 +1,18 @@ +sysinfo type='smbios' + processor +entry name='socket_destination'0/entry +entry name='type'ARMv7 Processor rev 5 (v7l)/entry + /processor + processor +entry name='socket_destination'1/entry +entry name='type'ARMv7 Processor rev 5 (v7l)/entry + /processor + processor +entry name='socket_destination'2/entry +entry name='type'ARMv7 Processor rev 5 (v7l)/entry + /processor + processor +entry name='socket_destination'3/entry +entry name='type'ARMv7 Processor rev 5 (v7l)/entry + /processor +/sysinfo diff --git a/tests/sysinfotest.c b/tests/sysinfotest.c index d8266a7..74e5f71 100644 --- a/tests/sysinfotest.c +++ b/tests/sysinfotest.c @@ -166,11 +166,23 @@ VIRT_TEST_MAIN(test_x86) static int test_arm(void) { -return sysinfotest_run(arm sysinfo, - NULL, - NULL, - /sysinfodata/armcpuinfo.data, - /sysinfodata/armsysinfo.expect); +int ret = EXIT_SUCCESS; + +if (sysinfotest_run(arm sysinfo, +NULL, +NULL, +/sysinfodata/armcpuinfo.data, +/sysinfodata/armsysinfo.expect) != EXIT_SUCCESS) +ret = EXIT_FAILURE; + +if
[libvirt] [PATCH 0/4] Misc clean-ups again
Just a couple of them this time. Martin Kletzander (4): rpc: Don't mix max_clients and max_workers in PostExecRestart virnetserver: Remove unnecessary double space gendispatch: Don't generate long lines Some alignment fixes in lxc_controller and jsontest src/lxc/lxc_controller.c | 16 src/rpc/gendispatch.pl | 5 +++-- src/rpc/virnetserver.c | 6 +++--- tests/jsontest.c | 12 ++-- 4 files changed, 20 insertions(+), 19 deletions(-) -- 2.4.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 1/5] Add privateData to virDomainDiskDef
Signed-off-by: Jiri Denemark jdene...@redhat.com --- Notes: Version 2: - new patch src/conf/domain_conf.c| 14 +- src/conf/domain_conf.h| 6 +- src/parallels/parallels_sdk.c | 4 ++-- src/qemu/qemu_command.c | 4 ++-- src/vbox/vbox_common.c| 4 ++-- src/vbox/vbox_tmpl.c | 6 +++--- src/vmx/vmx.c | 2 +- src/xenconfig/xen_sxpr.c | 6 +++--- src/xenconfig/xen_xl.c| 2 +- src/xenconfig/xen_xm.c| 4 ++-- 10 files changed, 30 insertions(+), 22 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index add857c..d3a9093 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1275,7 +1275,7 @@ void virDomainLeaseDefFree(virDomainLeaseDefPtr def) virDomainDiskDefPtr -virDomainDiskDefNew(void) +virDomainDiskDefNew(virDomainXMLOptionPtr xmlopt) { virDomainDiskDefPtr ret; @@ -1285,6 +1285,11 @@ virDomainDiskDefNew(void) if (VIR_ALLOC(ret-src) 0) goto error; +if (xmlopt +xmlopt-privateData.diskNew +!(ret-privateData = xmlopt-privateData.diskNew())) +goto error; + if (virCondInit(ret-blockJobSyncCond) 0) { virReportSystemError(errno, %s, _(Failed to initialize condition)); goto error; @@ -1293,9 +1298,7 @@ virDomainDiskDefNew(void) return ret; error: -virStorageSourceFree(ret-src); -VIR_FREE(ret); - +virDomainDiskDefFree(ret); return NULL; } @@ -1315,6 +1318,7 @@ virDomainDiskDefFree(virDomainDiskDefPtr def) VIR_FREE(def-product); VIR_FREE(def-domain_name); virDomainDeviceInfoClear(def-info); +virObjectUnref(def-privateData); virCondDestroy(def-blockJobSyncCond); VIR_FREE(def); @@ -6121,7 +6125,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, int auth_secret_usage = -1; int ret = 0; -if (!(def = virDomainDiskDefNew())) +if (!(def = virDomainDiskDefNew(xmlopt))) return NULL; def-geometry.cylinders = 0; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 2cd105a7..f57f3c9 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -683,6 +683,8 @@ typedef enum { struct _virDomainDiskDef { virStorageSourcePtr src; /* non-NULL. XXX Allow NULL for empty cdrom? */ +virObjectPtr privateData; + int device; /* enum virDomainDiskDevice */ int bus; /* enum virDomainDiskBus */ char *dst; @@ -2332,6 +2334,7 @@ typedef virDomainXMLOption *virDomainXMLOptionPtr; typedef void *(*virDomainXMLPrivateDataAllocFunc)(void); typedef void (*virDomainXMLPrivateDataFreeFunc)(void *); +typedef virObjectPtr (*virDomainXMLPrivateDataNewFunc)(void); typedef int (*virDomainXMLPrivateDataFormatFunc)(virBufferPtr, void *); typedef int (*virDomainXMLPrivateDataParseFunc)(xmlXPathContextPtr, void *); @@ -2368,6 +2371,7 @@ typedef virDomainXMLPrivateDataCallbacks *virDomainXMLPrivateDataCallbacksPtr; struct _virDomainXMLPrivateDataCallbacks { virDomainXMLPrivateDataAllocFunc alloc; virDomainXMLPrivateDataFreeFunc free; +virDomainXMLPrivateDataNewFuncdiskNew; virDomainXMLPrivateDataFormatFunc format; virDomainXMLPrivateDataParseFunc parse; }; @@ -2420,7 +2424,7 @@ void virDomainPanicDefFree(virDomainPanicDefPtr panic); void virDomainResourceDefFree(virDomainResourceDefPtr resource); void virDomainGraphicsDefFree(virDomainGraphicsDefPtr def); void virDomainInputDefFree(virDomainInputDefPtr def); -virDomainDiskDefPtr virDomainDiskDefNew(void); +virDomainDiskDefPtr virDomainDiskDefNew(virDomainXMLOptionPtr xmlopt); void virDomainDiskDefFree(virDomainDiskDefPtr def); void virDomainLeaseDefFree(virDomainLeaseDefPtr def); int virDomainDiskGetType(virDomainDiskDefPtr def); diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c index ad744c9..08d4a48 100644 --- a/src/parallels/parallels_sdk.c +++ b/src/parallels/parallels_sdk.c @@ -626,7 +626,7 @@ prlsdkAddDomainHardDisksInfo(PRL_HANDLE sdkdom, virDomainDefPtr def) PrlHandle_Free(hdd); hdd = PRL_INVALID_HANDLE; } else { -if (!(disk = virDomainDiskDefNew())) +if (!(disk = virDomainDiskDefNew(NULL))) goto error; if (prlsdkGetDiskInfo(hdd, disk, false) 0) @@ -666,7 +666,7 @@ prlsdkAddDomainOpticalDisksInfo(PRL_HANDLE sdkdom, virDomainDefPtr def) pret = PrlVmCfg_GetOpticalDisk(sdkdom, i, cdrom); prlsdkCheckRetGoto(pret, error); -if (!(disk = virDomainDiskDefNew())) +if (!(disk = virDomainDiskDefNew(NULL))) goto error; if (prlsdkGetDiskInfo(cdrom, disk, true) 0) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 5d0a167..2939f8d 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -12566,7 +12566,7 @@ qemuParseCommandLine(virCapsPtr qemuCaps, STRPREFIX(arg, -fd)
[libvirt] [PATCH v2 0/5] Cleanup storage migration code a bit
Version 2: - move QEMU-only data from virDomainDiskDef to a private data object Jiri Denemark (5): Add privateData to virDomainDiskDef Rename virDomainHasBlockjob as qemuDomainHasBlockjob Move QEMU-only fields from virDomainDiskDef into privateData qemu: Keep track of what disks are being migrated qemu: Don't give up on first error in qemuMigrationCancelDriverMirror src/conf/domain_conf.c| 43 --- src/conf/domain_conf.h| 19 +++--- src/libvirt_private.syms | 1 - src/parallels/parallels_sdk.c | 4 +-- src/qemu/qemu_blockjob.c | 48 ++--- src/qemu/qemu_command.c | 4 +-- src/qemu/qemu_domain.c| 81 ++- src/qemu/qemu_domain.h| 25 + src/qemu/qemu_driver.c| 15 src/qemu/qemu_migration.c | 42 +++--- src/qemu/qemu_process.c | 17 + src/vbox/vbox_common.c| 4 +-- src/vbox/vbox_tmpl.c | 6 ++-- src/vmx/vmx.c | 2 +- src/xenconfig/xen_sxpr.c | 6 ++-- src/xenconfig/xen_xl.c| 2 +- src/xenconfig/xen_xm.c| 4 +-- 17 files changed, 201 insertions(+), 122 deletions(-) -- 2.4.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 2/5] Rename virDomainHasBlockjob as qemuDomainHasBlockjob
And move it to qemu_domain.[ch] because this API is QEMU-only. Signed-off-by: Jiri Denemark jdene...@redhat.com --- Notes: Version 2: - new patch src/conf/domain_conf.c| 27 --- src/conf/domain_conf.h| 3 --- src/qemu/qemu_domain.c| 28 src/qemu/qemu_domain.h| 2 ++ src/qemu/qemu_driver.c| 4 ++-- src/qemu/qemu_migration.c | 2 +- 6 files changed, 33 insertions(+), 33 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d3a9093..3204140 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -12322,33 +12322,6 @@ virDomainDiskRemoveByName(virDomainDefPtr def, const char *name) return virDomainDiskRemove(def, idx); } -/** - * virDomainHasBlockjob: - * @vm: domain object - * @copy_only: Reject only block copy job - * - * Return true if @vm has at least one disk involved in a current block - * copy/commit/pull job. If @copy_only is true this returns true only if the - * disk is involved in a block copy. - * */ -bool -virDomainHasBlockjob(virDomainObjPtr vm, - bool copy_only) -{ -size_t i; -for (i = 0; i vm-def-ndisks; i++) { -if (!copy_only -vm-def-disks[i]-blockjob) -return true; - -if (vm-def-disks[i]-mirror -vm-def-disks[i]-mirrorJob == VIR_DOMAIN_BLOCK_JOB_TYPE_COPY) -return true; -} - -return false; -} - int virDomainNetInsert(virDomainDefPtr def, virDomainNetDefPtr net) { /* hostdev net devices must also exist in the hostdevs array */ diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index f57f3c9..b5e7617 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2697,9 +2697,6 @@ int virDomainDiskSourceParse(xmlNodePtr node, xmlXPathContextPtr ctxt, virStorageSourcePtr src); -bool virDomainHasBlockjob(virDomainObjPtr vm, - bool copy_only); - int virDomainNetFindIdx(virDomainDefPtr def, virDomainNetDefPtr net); virDomainNetDefPtr virDomainNetFind(virDomainDefPtr def, const char *device); bool virDomainHasNet(virDomainDefPtr def, virDomainNetDefPtr net); diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index fa8229f..b69f10f 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2828,6 +2828,34 @@ qemuDomainDiskBlockJobIsActive(virDomainDiskDefPtr disk) } +/** + * qemuDomainHasBlockjob: + * @vm: domain object + * @copy_only: Reject only block copy job + * + * Return true if @vm has at least one disk involved in a current block + * copy/commit/pull job. If @copy_only is true this returns true only if the + * disk is involved in a block copy. + * */ +bool +qemuDomainHasBlockjob(virDomainObjPtr vm, + bool copy_only) +{ +size_t i; +for (i = 0; i vm-def-ndisks; i++) { +if (!copy_only +vm-def-disks[i]-blockjob) +return true; + +if (vm-def-disks[i]-mirror +vm-def-disks[i]-mirrorJob == VIR_DOMAIN_BLOCK_JOB_TYPE_COPY) +return true; +} + +return false; +} + + int qemuDomainUpdateDeviceList(virQEMUDriverPtr driver, virDomainObjPtr vm, diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 3162f84..7f2e4b5 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -432,6 +432,8 @@ int qemuDomainJobInfoToParams(qemuDomainJobInfoPtr jobInfo, int qemuDomainSupportsBlockJobs(virDomainObjPtr vm, bool *modern) ATTRIBUTE_NONNULL(1); bool qemuDomainDiskBlockJobIsActive(virDomainDiskDefPtr disk); +bool qemuDomainHasBlockjob(virDomainObjPtr vm, bool copy_only) +ATTRIBUTE_NONNULL(1); int qemuDomainAlignMemorySizes(virDomainDefPtr def); void qemuDomainMemoryDeviceAlignSize(virDomainMemoryDefPtr mem); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f7433ee..c54199c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7734,7 +7734,7 @@ static virDomainPtr qemuDomainDefineXMLFlags(virConnectPtr conn, const char *xml virObjectRef(vm); def = NULL; -if (virDomainHasBlockjob(vm, true)) { +if (qemuDomainHasBlockjob(vm, true)) { virReportError(VIR_ERR_BLOCK_COPY_ACTIVE, %s, _(domain has active block job)); virDomainObjAssignDef(vm, NULL, false, NULL); @@ -15622,7 +15622,7 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, if (!(caps = virQEMUDriverGetCapabilities(driver, false))) goto cleanup; -if (virDomainHasBlockjob(vm, false)) { +if (qemuDomainHasBlockjob(vm, false)) { virReportError(VIR_ERR_OPERATION_INVALID, %s, _(domain has active block job)); goto cleanup; diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index c1af704..8f2189b 100644 --- a/src/qemu/qemu_migration.c +++
[libvirt] [PATCH v2 5/5] qemu: Don't give up on first error in qemuMigrationCancelDriverMirror
When cancelling drive mirror, always try to do that for all disks even if it fails for some of them. Report the first error we saw. Signed-off-by: Jiri Denemark jdene...@redhat.com --- Notes: Version 2: - rebased (context changed) src/qemu/qemu_migration.c | 15 --- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index c4f1c48..d12b7cc 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1862,6 +1862,8 @@ static int qemuMigrationCancelDriveMirror(virQEMUDriverPtr driver, virDomainObjPtr vm) { +virErrorPtr err = NULL; +int ret = 0; size_t i; for (i = 0; i vm-def-ndisks; i++) { @@ -1871,13 +1873,20 @@ qemuMigrationCancelDriveMirror(virQEMUDriverPtr driver, if (!diskPriv-migrating || !diskPriv-blockJobSync) continue; -if (qemuMigrationCancelOneDriveMirror(driver, vm, disk) 0) -return -1; +if (qemuMigrationCancelOneDriveMirror(driver, vm, disk) 0) { +ret = -1; +if (!err) +err = virSaveLastError(); +} diskPriv-migrating = false; } -return 0; +if (err) { +virSetError(err); +virFreeError(err); +} +return ret; } -- 2.4.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC v1 0/6] Live Migration with ephemeral host NIC devices
On 05/13/2015 10:42 AM, Dr. David Alan Gilbert wrote: * Laine Stump (la...@redhat.com) wrote: On 05/13/2015 04:28 AM, Peter Krempa wrote: On Wed, May 13, 2015 at 09:08:39 +0100, Dr. David Alan Gilbert wrote: * Peter Krempa (pkre...@redhat.com) wrote: On Wed, May 13, 2015 at 11:36:26 +0800, Chen Fan wrote: my main goal is to add support migration with host NIC passthrough devices and keep the network connectivity. this series patch base on Shradha's patches on https://www.redhat.com/archives/libvir-list/2012-November/msg01324.html which is add migration support for host passthrough devices. 1) unplug the ephemeral devices before migration 2) do native migration 3) when migration finished, hotplug the ephemeral devices IMHO this algorithm is something that an upper layer management app should do. The device unplug operation is complex and it might not succeed which will make the current migration thread hang or fail in an intermediate state that will not be recoverable. However you wouldn't want each of the upper layer management apps implementing their own hacks for this; so something somewhere needs to standardise what the guest sees. The guest still will see an PCI device unplug request and will have to respond to it, then will be paused and after resume a new PCI device will appear. This is standardised. The nonstandardised part (which can't really be standardised) is how the bonding or other guest-dependant stuff will be handled, but that is up to the guest OS to handle. From libvirt's perspective this is only something that will trigger the device unplug and plug the devices back. And there are a lot of issues here: 1) the destination of the migration might not have the desired devices This will trigger a lot of problems as we will not be able to guarantee that the devices reappear on the destination and if we'd wanted to check we'd need a new migration protocol AFAIK. 2) The guest OS might refuse to detach the PCI device (it might be stuck before PCI code is loaded) In that case the migration will be stuck forever and abort attempts will make the domain state basically undefined depending on the phase where it failed. Since we can't guarantee that the unplug of the PCI host devices will be atomic or that it will succeed we basically can't guarantee in any way in which state the VM will end up later after (a possibly failed) migration. To recover such state there are too many option that could be desired by the user that would be hard to implement in a way that would be flexible enough. In the past I've been on the side of having libvirt automatically do the device detach and reattach (but definitely on the side of the guest agent and libvirt keeping their hands off of network configuration in the guest), with the thinking that 1) libvirt is in a well situated spot to do it, and 2) this would eliminate duplicate code in the upper level management. However, Peter's points above made me consider the failure cases more closely, in particular this one: * the destination claims to have the resources required (right type of PCI device, enough RAM), so migration is started. * device detached on source, guest memory migrated to destination, * guest started - no problems. (At this point, since the guest has been restarted, it's not really possible for libvirt to fail the migration in a recoverable manner (unless you want to implement some sort of unmigration so that the guest state on the source is updated with whatever execution occurred on the destination, and I don't think *anyone* wants to go there)) * libvirt finds the device still available and attempts to attach it but (for some odd reason) fails. Now libvirt can't tell the application that the migration has succeeded, because it didn't (unless the device was marked as optional), but it also can't fail the migration except to say this is such a monumental failure that your guest has simply died. If, on the other hand, the detach and re-attach are implemented in a higher layer (ovirt/openstack), they will at least have the guest in a state they can deal with - it won't be pretty, but they could for example migrate the guest to another host (maybe back to the source) and re-attach there. So this one message from Peter has nicely pointed out the error in my thinking, and I now agree that auto-detach/reattach shouldn't be implemented in libvirt - it would work nicely in an error free world, but would crumble in the face of some errors. (I just wish I had considered the particular failure mode above a year or two ago, so I could have been more discouraging in my emails then :-) It's a shame to limit the utility of this by dealing with an error case that's not a fatal error. Does libvirt not have a way of dealing with non-fatal errors? But is it non-fatal? Dan's point is that isn't up to libvirt to decide. In the case of
[libvirt] [PATCH v2] parallels: remove connection wide wait timeout
We have a lot of passing arguments code just to pass connection object cause it holds jobTimeout. Taking into account that right now this value is defined at compile time let's just get rid of it and make arguments list more clear in many places. In case we later need some runtime configurable timeout value we can provide this value through arguments function already operate such as a parallels domain object etc as this timeouts are operation( and thus object) specific in practice. Signed-off-by: Nikolay Shirokovskiy nshirokovs...@parallels.com --- src/parallels/parallels_driver.c |9 ++-- src/parallels/parallels_sdk.c| 108 +- src/parallels/parallels_sdk.h| 27 - src/parallels/parallels_utils.h |1 - 4 files changed, 64 insertions(+), 81 deletions(-) diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c index 205ec1c..4b87213 100644 --- a/src/parallels/parallels_driver.c +++ b/src/parallels/parallels_driver.c @@ -214,7 +214,7 @@ parallelsOpenDefault(virConnectPtr conn) goto err_free; } -if (prlsdkInit(privconn)) { +if (prlsdkInit()) { VIR_DEBUG(%s, _(Can't initialize Parallels SDK)); goto err_free; } @@ -1007,7 +1007,6 @@ parallelsDomainManagedSave(virDomainPtr domain, unsigned int flags) static int parallelsDomainManagedSaveRemove(virDomainPtr domain, unsigned int flags) { -parallelsConnPtr privconn = domain-conn-privateData; virDomainObjPtr dom = NULL; int state, reason; int ret = -1; @@ -1022,7 +1021,7 @@ parallelsDomainManagedSaveRemove(virDomainPtr domain, unsigned int flags) if (!(state == VIR_DOMAIN_SHUTOFF reason == VIR_DOMAIN_SHUTOFF_SAVED)) goto cleanup; -ret = prlsdkDomainManagedSaveRemove(privconn, dom); +ret = prlsdkDomainManagedSaveRemove(dom); cleanup: virObjectUnlock(dom); @@ -1071,7 +1070,7 @@ static int parallelsDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, switch (dev-type) { case VIR_DOMAIN_DEVICE_DISK: -ret = prlsdkAttachVolume(dom-conn, privdom, dev-data.disk); +ret = prlsdkAttachVolume(privdom, dev-data.disk); if (ret) { virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(disk attach failed)); @@ -1140,7 +1139,7 @@ static int parallelsDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, switch (dev-type) { case VIR_DOMAIN_DEVICE_DISK: -ret = prlsdkDetachVolume(dom-conn, privdom, dev-data.disk); +ret = prlsdkDetachVolume(privdom, dev-data.disk); if (ret) { virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(disk detach failed)); diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c index fcd274f..d48d4b7 100644 --- a/src/parallels/parallels_sdk.c +++ b/src/parallels/parallels_sdk.c @@ -35,8 +35,6 @@ #define VIR_FROM_THIS VIR_FROM_PARALLELS #define JOB_INFINIT_WAIT_TIMEOUT UINT_MAX -PRL_UINT32 defaultJobTimeout = JOB_INFINIT_WAIT_TIMEOUT; - VIR_LOG_INIT(parallels.sdk); /* @@ -179,9 +177,9 @@ getJobResultHelper(PRL_HANDLE job, unsigned int timeout, PRL_HANDLE *result, return ret; } -#define getJobResult(job, timeout, result) \ -getJobResultHelper(job, timeout, result, __FILE__, \ - __FUNCTION__, __LINE__) +#define getJobResult(job, result) \ +getJobResultHelper(job, JOB_INFINIT_WAIT_TIMEOUT, \ +result, __FILE__, __FUNCTION__, __LINE__) static PRL_RESULT waitJobHelper(PRL_HANDLE job, unsigned int timeout, @@ -197,12 +195,13 @@ waitJobHelper(PRL_HANDLE job, unsigned int timeout, return ret; } -#define waitJob(job, timeout) \ -waitJobHelper(job, timeout, __FILE__, \ +#define waitJob(job)\ +waitJobHelper(job, JOB_INFINIT_WAIT_TIMEOUT, __FILE__, \ __FUNCTION__, __LINE__) + int -prlsdkInit(parallelsConnPtr privconn) +prlsdkInit(void) { PRL_RESULT ret; @@ -212,8 +211,6 @@ prlsdkInit(parallelsConnPtr privconn) return -1; } -privconn-jobTimeout = JOB_INFINIT_WAIT_TIMEOUT; - return 0; }; @@ -238,7 +235,7 @@ prlsdkConnect(parallelsConnPtr privconn) job = PrlSrv_LoginLocalEx(privconn-server, NULL, 0, PSL_HIGH_SECURITY, PACF_NON_INTERACTIVE_MODE); -if (waitJob(job, privconn-jobTimeout)) { +if (waitJob(job)) { PrlHandle_Free(privconn-server); return -1; } @@ -252,7 +249,7 @@ prlsdkDisconnect(parallelsConnPtr privconn) PRL_HANDLE job; job = PrlSrv_Logoff(privconn-server); -waitJob(job, privconn-jobTimeout); +waitJob(job); PrlHandle_Free(privconn-server); } @@ -269,7 +266,7 @@ prlsdkSdkDomainLookup(parallelsConnPtr privconn, int ret = -1; job =
Re: [libvirt] [PATCH] Ignore Vim swap files.
On 05/10/2015 03:28 PM, Cole Robinson wrote: On 05/05/2015 07:34 AM, Andrea Bolognani wrote: This removes some noise when you're working on the repository and also have a bunch of source files open in Vim in another terminal. --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 7bcf359..1a5cf8e 100644 --- a/.gitignore +++ b/.gitignore @@ -18,6 +18,7 @@ *.pyc *.rej *.s +*.swp *~ .#* .deps Instead of adding more bits to the .gitignore that aren't specific to the project, I'd recommend you set up a global .gitignore to ignore editor bits: I generally recommend that (or at least a per-project local ignore: echo '*.swp' .git/info/exclude ), but in this particular case, since it is right next to emacs *~ and .*# ignores, I'm okay with taking this patch. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC v1 4/6] migration: Migration support for ephemeral hostdevs
On 05/13/2015 04:36 AM, Peter Krempa wrote: On Wed, May 13, 2015 at 11:36:30 +0800, Chen Fan wrote: add migration support for ephemeral host devices, introduce two 'detach' and 'restore' functions to unplug/plug host devices during migration. Signed-off-by: Chen Fan chen.fan.f...@cn.fujitsu.com --- src/qemu/qemu_migration.c | 171 -- src/qemu/qemu_migration.h | 9 +++ src/qemu/qemu_process.c | 11 +++ 3 files changed, 187 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 56112f9..d5a698f 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3384,6 +3384,158 @@ qemuMigrationPrepareDef(virQEMUDriverPtr driver, return def; } +int +qemuMigrationDetachEphemeralDevices(virQEMUDriverPtr driver, +virDomainObjPtr vm, +bool live) +{ +qemuDomainObjPrivatePtr priv = vm-privateData; +virDomainHostdevDefPtr hostdev; +virDomainNetDefPtr net; +virDomainDeviceDef dev; +virDomainDeviceDefPtr dev_copy = NULL; +virCapsPtr caps = NULL; +int actualType; +int ret = -1; +size_t i; + +VIR_DEBUG(Rum domain detach ephemeral devices); + +if (!(caps = virQEMUDriverGetCapabilities(driver, false))) +return ret; + +for (i = 0; i vm-def-nnets;) { +net = vm-def-nets[i]; + +actualType = virDomainNetGetActualType(net); +if (actualType != VIR_DOMAIN_NET_TYPE_HOSTDEV) { +i++; +continue; +} + +hostdev = virDomainNetGetActualHostdev(net); +if (hostdev-mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS || +hostdev-source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI || +!hostdev-ephemeral) { +i++; +continue; +} + +dev.type = VIR_DOMAIN_DEVICE_NET; +dev.data.net = net; + +dev_copy = virDomainDeviceDefCopy(dev, vm-def, + caps, driver-xmlopt); +if (!dev_copy) +goto cleanup; + +if (live) { +/* nnets reduced */ +if (qemuDomainDetachNetDevice(driver, vm, dev_copy) 0) +goto cleanup; So this is where the fun begins. qemuDomainDetachNetDevice is not designed to be called this way since the detach API where it's used normally returns 0 in the following two cases: 1) The detach was successfull, the guest removed the device 2) The detach request was successful, but guest did not remove the device yet In the latter case you need to wait for a event to successfully know when the device was removed. For historical reference: omission of this bit (needing to wait for the guest to remove the device) was one of the reasons Shradha's patches couldn't be pushed. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2] Ignore bridge template names with multiple printf conversions
On 05/13/2015 04:55 AM, Ján Tomko wrote: Okay, I see your counterargument. Still, strstr() is pretty expensive compared to just: if (def-bridge (p = strchr(def-bridge, '%')) == strrchr(def-bridge, '%') p[1] == 'd') Coverity complains : Event returned_null: strchr returns null (checked 273 out of 288 times). strchr does not return NULL here because networkFindUnusedBridgeName is only called if either def-bridge is NULL or def-bridge contains %d. Adding p just before p[1] == 'd' should silence the false positive, right? -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC v1 0/6] Live Migration with ephemeral host NIC devices
* Laine Stump (la...@redhat.com) wrote: On 05/13/2015 04:28 AM, Peter Krempa wrote: On Wed, May 13, 2015 at 09:08:39 +0100, Dr. David Alan Gilbert wrote: * Peter Krempa (pkre...@redhat.com) wrote: On Wed, May 13, 2015 at 11:36:26 +0800, Chen Fan wrote: my main goal is to add support migration with host NIC passthrough devices and keep the network connectivity. this series patch base on Shradha's patches on https://www.redhat.com/archives/libvir-list/2012-November/msg01324.html which is add migration support for host passthrough devices. 1) unplug the ephemeral devices before migration 2) do native migration 3) when migration finished, hotplug the ephemeral devices IMHO this algorithm is something that an upper layer management app should do. The device unplug operation is complex and it might not succeed which will make the current migration thread hang or fail in an intermediate state that will not be recoverable. However you wouldn't want each of the upper layer management apps implementing their own hacks for this; so something somewhere needs to standardise what the guest sees. The guest still will see an PCI device unplug request and will have to respond to it, then will be paused and after resume a new PCI device will appear. This is standardised. The nonstandardised part (which can't really be standardised) is how the bonding or other guest-dependant stuff will be handled, but that is up to the guest OS to handle. From libvirt's perspective this is only something that will trigger the device unplug and plug the devices back. And there are a lot of issues here: 1) the destination of the migration might not have the desired devices This will trigger a lot of problems as we will not be able to guarantee that the devices reappear on the destination and if we'd wanted to check we'd need a new migration protocol AFAIK. 2) The guest OS might refuse to detach the PCI device (it might be stuck before PCI code is loaded) In that case the migration will be stuck forever and abort attempts will make the domain state basically undefined depending on the phase where it failed. Since we can't guarantee that the unplug of the PCI host devices will be atomic or that it will succeed we basically can't guarantee in any way in which state the VM will end up later after (a possibly failed) migration. To recover such state there are too many option that could be desired by the user that would be hard to implement in a way that would be flexible enough. In the past I've been on the side of having libvirt automatically do the device detach and reattach (but definitely on the side of the guest agent and libvirt keeping their hands off of network configuration in the guest), with the thinking that 1) libvirt is in a well situated spot to do it, and 2) this would eliminate duplicate code in the upper level management. However, Peter's points above made me consider the failure cases more closely, in particular this one: * the destination claims to have the resources required (right type of PCI device, enough RAM), so migration is started. * device detached on source, guest memory migrated to destination, * guest started - no problems. (At this point, since the guest has been restarted, it's not really possible for libvirt to fail the migration in a recoverable manner (unless you want to implement some sort of unmigration so that the guest state on the source is updated with whatever execution occurred on the destination, and I don't think *anyone* wants to go there)) * libvirt finds the device still available and attempts to attach it but (for some odd reason) fails. Now libvirt can't tell the application that the migration has succeeded, because it didn't (unless the device was marked as optional), but it also can't fail the migration except to say this is such a monumental failure that your guest has simply died. If, on the other hand, the detach and re-attach are implemented in a higher layer (ovirt/openstack), they will at least have the guest in a state they can deal with - it won't be pretty, but they could for example migrate the guest to another host (maybe back to the source) and re-attach there. So this one message from Peter has nicely pointed out the error in my thinking, and I now agree that auto-detach/reattach shouldn't be implemented in libvirt - it would work nicely in an error free world, but would crumble in the face of some errors. (I just wish I had considered the particular failure mode above a year or two ago, so I could have been more discouraging in my emails then :-) It's a shame to limit the utility of this by dealing with an error case that's not a fatal error. Does libvirt not have a way of dealing with non-fatal errors? Dave -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK --
Re: [libvirt] [PATCH] qemu: clear useless code
On 13.05.2015 07:56, zhang bo wrote: The variable 'now' is useless in qemuMigrationPrepareAny(), so I clear it. Signed-off-by: Wang Yufei james.wangyu...@huawei.com Signed-off-by: Zhang Bo oscar.zhan...@huawei.com --- src/qemu/qemu_migration.c | 4 1 file changed, 4 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index c1af704..4dcba7a 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2913,7 +2913,6 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, int ret = -1; int dataFD[2] = { -1, -1 }; qemuDomainObjPrivatePtr priv = NULL; -unsigned long long now; qemuMigrationCookiePtr mig = NULL; bool tunnel = !!st; char *xmlout = NULL; @@ -2923,9 +2922,6 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, bool abort_on_error = !!(flags VIR_MIGRATE_ABORT_ON_ERROR); bool taint_hook = false; -if (virTimeMillisNow(now) 0) -return -1; - virNWFilterReadLockFilterUpdates(); if (flags VIR_MIGRATE_OFFLINE) { I've reworded the commit message a bit, ACKed and pushed. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Ignore Vim swap files.
On 05/05/2015 05:34 AM, Andrea Bolognani wrote: This removes some noise when you're working on the repository and also have a bunch of source files open in Vim in another terminal. --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 7bcf359..1a5cf8e 100644 --- a/.gitignore +++ b/.gitignore @@ -18,6 +18,7 @@ *.pyc *.rej *.s +*.swp *~ .#* ACK and pushed, on the grounds that we already have similar treatment for emacs working files. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2] Ignore bridge template names with multiple printf conversions
On 05/13/2015 10:58 AM, Eric Blake wrote: On 05/13/2015 04:55 AM, Ján Tomko wrote: Okay, I see your counterargument. Still, strstr() is pretty expensive compared to just: if (def-bridge (p = strchr(def-bridge, '%')) == strrchr(def-bridge, '%') p[1] == 'd') Coverity complains : Event returned_null:strchr returns null (checked 273 out of 288 times). strchr does not return NULL here because networkFindUnusedBridgeName is only called if either def-bridge is NULL or def-bridge contains %d. Adding p just before p[1] == 'd' should silence the false positive, right? Yes. I'll send something in a bit - there's another two that are queued up as well. John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC v1 0/6] Live Migration with ephemeral host NIC devices
On 05/13/2015 04:28 AM, Peter Krempa wrote: On Wed, May 13, 2015 at 09:08:39 +0100, Dr. David Alan Gilbert wrote: * Peter Krempa (pkre...@redhat.com) wrote: On Wed, May 13, 2015 at 11:36:26 +0800, Chen Fan wrote: my main goal is to add support migration with host NIC passthrough devices and keep the network connectivity. this series patch base on Shradha's patches on https://www.redhat.com/archives/libvir-list/2012-November/msg01324.html which is add migration support for host passthrough devices. 1) unplug the ephemeral devices before migration 2) do native migration 3) when migration finished, hotplug the ephemeral devices IMHO this algorithm is something that an upper layer management app should do. The device unplug operation is complex and it might not succeed which will make the current migration thread hang or fail in an intermediate state that will not be recoverable. However you wouldn't want each of the upper layer management apps implementing their own hacks for this; so something somewhere needs to standardise what the guest sees. The guest still will see an PCI device unplug request and will have to respond to it, then will be paused and after resume a new PCI device will appear. This is standardised. The nonstandardised part (which can't really be standardised) is how the bonding or other guest-dependant stuff will be handled, but that is up to the guest OS to handle. From libvirt's perspective this is only something that will trigger the device unplug and plug the devices back. And there are a lot of issues here: 1) the destination of the migration might not have the desired devices This will trigger a lot of problems as we will not be able to guarantee that the devices reappear on the destination and if we'd wanted to check we'd need a new migration protocol AFAIK. 2) The guest OS might refuse to detach the PCI device (it might be stuck before PCI code is loaded) In that case the migration will be stuck forever and abort attempts will make the domain state basically undefined depending on the phase where it failed. Since we can't guarantee that the unplug of the PCI host devices will be atomic or that it will succeed we basically can't guarantee in any way in which state the VM will end up later after (a possibly failed) migration. To recover such state there are too many option that could be desired by the user that would be hard to implement in a way that would be flexible enough. In the past I've been on the side of having libvirt automatically do the device detach and reattach (but definitely on the side of the guest agent and libvirt keeping their hands off of network configuration in the guest), with the thinking that 1) libvirt is in a well situated spot to do it, and 2) this would eliminate duplicate code in the upper level management. However, Peter's points above made me consider the failure cases more closely, in particular this one: * the destination claims to have the resources required (right type of PCI device, enough RAM), so migration is started. * device detached on source, guest memory migrated to destination, * guest started - no problems. (At this point, since the guest has been restarted, it's not really possible for libvirt to fail the migration in a recoverable manner (unless you want to implement some sort of unmigration so that the guest state on the source is updated with whatever execution occurred on the destination, and I don't think *anyone* wants to go there)) * libvirt finds the device still available and attempts to attach it but (for some odd reason) fails. Now libvirt can't tell the application that the migration has succeeded, because it didn't (unless the device was marked as optional), but it also can't fail the migration except to say this is such a monumental failure that your guest has simply died. If, on the other hand, the detach and re-attach are implemented in a higher layer (ovirt/openstack), they will at least have the guest in a state they can deal with - it won't be pretty, but they could for example migrate the guest to another host (maybe back to the source) and re-attach there. So this one message from Peter has nicely pointed out the error in my thinking, and I now agree that auto-detach/reattach shouldn't be implemented in libvirt - it would work nicely in an error free world, but would crumble in the face of some errors. (I just wish I had considered the particular failure mode above a year or two ago, so I could have been more discouraging in my emails then :-) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC v1 4/6] migration: Migration support for ephemeral hostdevs
On 05/13/2015 05:57 AM, Daniel P. Berrange wrote: On Wed, May 13, 2015 at 11:36:30AM +0800, Chen Fan wrote: add migration support for ephemeral host devices, introduce two 'detach' and 'restore' functions to unplug/plug host devices during migration. Signed-off-by: Chen Fan chen.fan.f...@cn.fujitsu.com --- src/qemu/qemu_migration.c | 171 -- src/qemu/qemu_migration.h | 9 +++ src/qemu/qemu_process.c | 11 +++ 3 files changed, 187 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 56112f9..d5a698f 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c +void +qemuMigrationRestoreEphemeralDevices(virQEMUDriverPtr driver, + virConnectPtr conn, + virDomainObjPtr vm, + bool live) +{ +qemuDomainObjPrivatePtr priv = vm-privateData; +virDomainDeviceDefPtr dev; +int ret = -1; +size_t i; + +VIR_DEBUG(Rum domain restore ephemeral devices); + +for (i = 0; i priv-nEphemeralDevices; i++) { +dev = priv-ephemeralDevices[i]; + +switch ((virDomainDeviceType) dev-type) { +case VIR_DOMAIN_DEVICE_NET: +if (live) { +ret = qemuDomainAttachNetDevice(conn, driver, vm, +dev-data.net); +} else { +ret = virDomainNetInsert(vm-def, dev-data.net); +} + +if (!ret) +dev-data.net = NULL; +break; +case VIR_DOMAIN_DEVICE_HOSTDEV: +if (live) { +ret = qemuDomainAttachHostDevice(conn, driver, vm, + dev-data.hostdev); + } else { +ret =virDomainHostdevInsert(vm-def, dev-data.hostdev); +} This re-attach step is where we actually have far far far worse problems than with detach. This is blindly assuming that the guest on the target host can use the same hostdev that it was using on the source host. (kind of pointless to comment on, since pkrempa has changed my opinion by forcing me to think about the failure to reattach condition, but could be useful info for others) For a hostdev, yes, but not for interface type='network' (which would point to a libvirt network pool of VFs). This is essentially useless in the real world. Agreed (for plain hostdev) Even if the same vendor/model device is available on the target host, it is very unlikely to be available at the same bus/slot/function that it was on the source. It is quite likely neccessary to allocate a complete different NIC, or if using SRIOV allocate a different function. It is also not uncommon to have different vendor/models, so a completely different NIC may be required. In the case of a network device, a different brand/model of NIC at a different PCI address using a different guest driver shouldn't be a problem for the guest, as long as the MAC address is the same (for a Linux guest anyway; not sure what a Windows guest would do with a NIC that had the same MAC but used a different driver). This points out the folly of trying to do migration with attached hostdevs (managed at *any* level), for anything other than SRIOV VFs (which can have their MAC address set before attach, unlike non-SRIOV NICs). -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] storage: Resolve Coverity FORWARD_NULL
On Wed, May 13, 2015 at 12:32:20 -0400, John Ferlan wrote: Coverity points out it's possible for one of the virCommand{Output|Error}* API's to have not allocated 'output' and/or 'error' in which case the strstr comparison will cause a NULL deref Signed-off-by: John Ferlan jfer...@redhat.com --- src/storage/storage_backend_disk.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c index 6394dac..5c2c49a 100644 --- a/src/storage/storage_backend_disk.c +++ b/src/storage/storage_backend_disk.c @@ -412,7 +412,7 @@ virStorageBackendDiskFindLabel(const char* device) /* if parted succeeds we have a valid partition table */ ret = virCommandRun(cmd, NULL); -if (ret 0) { +if (ret 0 output error) { if (strstr(output, unrecognised disk label) || strstr(error, unrecognised disk label)) { ret = 1; This doesn't seem to be correct if either output or error is NULL and the other one is non-NULL. I'm too lazy to check if it's possible or not, but I think we should change this code in the following way and be safe: if (ret 0) { if ((output strstr(output, unrecognised disk label)) || (error strstr(error, unrecognised disk label))) { ret = 1; Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC v1 0/6] Live Migration with ephemeral host NIC devices
* Laine Stump (la...@redhat.com) wrote: On 05/13/2015 10:42 AM, Dr. David Alan Gilbert wrote: * Laine Stump (la...@redhat.com) wrote: On 05/13/2015 04:28 AM, Peter Krempa wrote: On Wed, May 13, 2015 at 09:08:39 +0100, Dr. David Alan Gilbert wrote: * Peter Krempa (pkre...@redhat.com) wrote: On Wed, May 13, 2015 at 11:36:26 +0800, Chen Fan wrote: my main goal is to add support migration with host NIC passthrough devices and keep the network connectivity. this series patch base on Shradha's patches on https://www.redhat.com/archives/libvir-list/2012-November/msg01324.html which is add migration support for host passthrough devices. 1) unplug the ephemeral devices before migration 2) do native migration 3) when migration finished, hotplug the ephemeral devices IMHO this algorithm is something that an upper layer management app should do. The device unplug operation is complex and it might not succeed which will make the current migration thread hang or fail in an intermediate state that will not be recoverable. However you wouldn't want each of the upper layer management apps implementing their own hacks for this; so something somewhere needs to standardise what the guest sees. The guest still will see an PCI device unplug request and will have to respond to it, then will be paused and after resume a new PCI device will appear. This is standardised. The nonstandardised part (which can't really be standardised) is how the bonding or other guest-dependant stuff will be handled, but that is up to the guest OS to handle. From libvirt's perspective this is only something that will trigger the device unplug and plug the devices back. And there are a lot of issues here: 1) the destination of the migration might not have the desired devices This will trigger a lot of problems as we will not be able to guarantee that the devices reappear on the destination and if we'd wanted to check we'd need a new migration protocol AFAIK. 2) The guest OS might refuse to detach the PCI device (it might be stuck before PCI code is loaded) In that case the migration will be stuck forever and abort attempts will make the domain state basically undefined depending on the phase where it failed. Since we can't guarantee that the unplug of the PCI host devices will be atomic or that it will succeed we basically can't guarantee in any way in which state the VM will end up later after (a possibly failed) migration. To recover such state there are too many option that could be desired by the user that would be hard to implement in a way that would be flexible enough. In the past I've been on the side of having libvirt automatically do the device detach and reattach (but definitely on the side of the guest agent and libvirt keeping their hands off of network configuration in the guest), with the thinking that 1) libvirt is in a well situated spot to do it, and 2) this would eliminate duplicate code in the upper level management. However, Peter's points above made me consider the failure cases more closely, in particular this one: * the destination claims to have the resources required (right type of PCI device, enough RAM), so migration is started. * device detached on source, guest memory migrated to destination, * guest started - no problems. (At this point, since the guest has been restarted, it's not really possible for libvirt to fail the migration in a recoverable manner (unless you want to implement some sort of unmigration so that the guest state on the source is updated with whatever execution occurred on the destination, and I don't think *anyone* wants to go there)) * libvirt finds the device still available and attempts to attach it but (for some odd reason) fails. Now libvirt can't tell the application that the migration has succeeded, because it didn't (unless the device was marked as optional), but it also can't fail the migration except to say this is such a monumental failure that your guest has simply died. If, on the other hand, the detach and re-attach are implemented in a higher layer (ovirt/openstack), they will at least have the guest in a state they can deal with - it won't be pretty, but they could for example migrate the guest to another host (maybe back to the source) and re-attach there. So this one message from Peter has nicely pointed out the error in my thinking, and I now agree that auto-detach/reattach shouldn't be implemented in libvirt - it would work nicely in an error free world, but would crumble in the face of some errors. (I just wish I had considered the particular failure mode above a year or two ago, so I could have been more discouraging in my emails then :-) It's a shame to limit the utility of this by dealing with an error case that's not a fatal error.
[libvirt] [PATCH 0/3] Patches for a couple of Coverity errors
The first one is one I forgot the last time I sent Coverity patches a few weeks ago. The next two are from changes made recently. John Ferlan (3): storage: Resolve Coverity FORWARD_NULL conf: Resolve Coverity FORWARD_NULL network: Resolve Coverity FORWARD_NULL src/conf/domain_conf.c | 2 ++ src/network/bridge_driver.c| 2 +- src/storage/storage_backend_disk.c | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] storage: Resolve Coverity FORWARD_NULL
Coverity points out it's possible for one of the virCommand{Output|Error}* API's to have not allocated 'output' and/or 'error' in which case the strstr comparison will cause a NULL deref Signed-off-by: John Ferlan jfer...@redhat.com --- src/storage/storage_backend_disk.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c index 6394dac..5c2c49a 100644 --- a/src/storage/storage_backend_disk.c +++ b/src/storage/storage_backend_disk.c @@ -412,7 +412,7 @@ virStorageBackendDiskFindLabel(const char* device) /* if parted succeeds we have a valid partition table */ ret = virCommandRun(cmd, NULL); -if (ret 0) { +if (ret 0 output error) { if (strstr(output, unrecognised disk label) || strstr(error, unrecognised disk label)) { ret = 1; -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC v1 4/6] migration: Migration support for ephemeral hostdevs
On 05/13/2015 10:30 PM, Laine Stump wrote: On 05/13/2015 05:57 AM, Daniel P. Berrange wrote: On Wed, May 13, 2015 at 11:36:30AM +0800, Chen Fan wrote: add migration support for ephemeral host devices, introduce two 'detach' and 'restore' functions to unplug/plug host devices during migration. Signed-off-by: Chen Fan chen.fan.f...@cn.fujitsu.com --- src/qemu/qemu_migration.c | 171 -- src/qemu/qemu_migration.h | 9 +++ src/qemu/qemu_process.c | 11 +++ 3 files changed, 187 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 56112f9..d5a698f 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c +void +qemuMigrationRestoreEphemeralDevices(virQEMUDriverPtr driver, + virConnectPtr conn, + virDomainObjPtr vm, + bool live) +{ +qemuDomainObjPrivatePtr priv = vm-privateData; +virDomainDeviceDefPtr dev; +int ret = -1; +size_t i; + +VIR_DEBUG(Rum domain restore ephemeral devices); + +for (i = 0; i priv-nEphemeralDevices; i++) { +dev = priv-ephemeralDevices[i]; + +switch ((virDomainDeviceType) dev-type) { +case VIR_DOMAIN_DEVICE_NET: +if (live) { +ret = qemuDomainAttachNetDevice(conn, driver, vm, +dev-data.net); +} else { +ret = virDomainNetInsert(vm-def, dev-data.net); +} + +if (!ret) +dev-data.net = NULL; +break; +case VIR_DOMAIN_DEVICE_HOSTDEV: +if (live) { +ret = qemuDomainAttachHostDevice(conn, driver, vm, + dev-data.hostdev); + } else { +ret =virDomainHostdevInsert(vm-def, dev-data.hostdev); +} This re-attach step is where we actually have far far far worse problems than with detach. This is blindly assuming that the guest on the target host can use the same hostdev that it was using on the source host. (kind of pointless to comment on, since pkrempa has changed my opinion by forcing me to think about the failure to reattach condition, but could be useful info for others) For a hostdev, yes, but not for interface type='network' (which would point to a libvirt network pool of VFs). This is essentially useless in the real world. Agreed (for plain hostdev) Even if the same vendor/model device is available on the target host, it is very unlikely to be available at the same bus/slot/function that it was on the source. It is quite likely neccessary to allocate a complete different NIC, or if using SRIOV allocate a different function. It is also not uncommon to have different vendor/models, so a completely different NIC may be required. In the case of a network device, a different brand/model of NIC at a different PCI address using a different guest driver shouldn't be a problem for the guest, as long as the MAC address is the same (for a Linux guest anyway; not sure what a Windows guest would do with a NIC that had the same MAC but used a different driver). This points out the folly of trying to do migration with attached hostdevs (managed at *any* level), for anything other than SRIOV VFs (which can have their MAC address set before attach, unlike non-SRIOV NICs). . So should we focus on implementing the feature that support migration with SRIOV VFs at first? I think that is simple to achieve my original target that implement NIC passthrough device migration. because sometimes we assign a native NIC to guest to keep the performance of network I/O, due to the MAC limitation of the non-SRIOV NICs, as laine said the cost of SRIOV NIC is cheaper than what we try. Thanks, Chen -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC v1 3/6] qemu: add check ephemeral devices only for PCI host devices
On 05/13/2015 04:17 PM, Peter Krempa wrote: On Wed, May 13, 2015 at 11:36:29 +0800, Chen Fan wrote: currently, we only support PCI host devices with ephemeral flag. and USB already supports migration. so maybe in the near future we can support SCSI. Signed-off-by: Chen Fan chen.fan.f...@cn.fujitsu.com --- src/qemu/qemu_command.c | 10 ++ src/qemu/qemu_migration.c | 11 +++ 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index fc81214..5acd8b4 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -10182,6 +10182,16 @@ qemuBuildCommandLine(virConnectPtr conn, /* PCI */ if (hostdev-mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS +hostdev-source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB +(hostdev-source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI + hostdev-ephemeral)) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, + _(non-USB and non-PCI device assignment with ephemeral + flag are not supported by this version of qemu)); This functionality is not based on qemu support but on libvirt implementation so the error message is incorrect. indeed. thanks for pointing out this. Chen +goto error; +} + +if (hostdev-mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS hostdev-source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) { int backend = hostdev-source.subsys.u.pci.backend; diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 83be435..56112f9 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1981,21 +1981,24 @@ qemuMigrationIsAllowed(virQEMUDriverPtr driver, virDomainObjPtr vm, def = vm-def; } -/* Migration with USB host devices is allowed, all other devices are - * forbidden. +/* + * Migration with USB and ephemeral PCI host devices host devices are allowed, + * all other devices are forbidden. */ forbid = false; for (i = 0; i def-nhostdevs; i++) { virDomainHostdevDefPtr hostdev = def-hostdevs[i]; if (hostdev-mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS || -hostdev-source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) { +(hostdev-source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB +!hostdev-ephemeral)) { forbid = true; break; } } if (forbid) { virReportError(VIR_ERR_OPERATION_INVALID, %s, - _(domain has assigned non-USB host devices)); + _(domain has assigned non-USB and + non-ephemeral host devices)); return false; } This patch has to be moved after you actually implement the ephemeral device unplug code, since an intermediate state would allow to bypass the check while the devices actually would not be unplugged. Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] cpuset / numa and qemu in TCG mode
Quoting Guido Günther (a...@sigxcpu.org): On Tue, May 12, 2015 at 11:14:09AM +0200, Martin Kletzander wrote: On Tue, May 12, 2015 at 05:27:34PM +1000, Tony Breeds wrote: On Mon, May 11, 2015 at 01:14:58PM +0200, Martin Kletzander wrote: Determining this by version might not be reliable, but more importantly working around bug in underlying software is something that shouldn't be done at all IMHO. Let the maintainers backport whatever needs to be done. I agree with you in an ideal world but there are times when we do need to add work arounds in $project_x to work around issues in $project_y. Ther nova side will be pretty easy regardless. I'd say the best solution would be to back port the 'fix' but that seems like a lot of effort given the number of distros and libvirt versions potentiall involved. If you want the fix to be distro-agnostic, there's nothing easier than back-porting the fix into our upstream maintenance branches. Those should make the life of distro maintainers easy (although it looks like not many distros use it). And this is part of the problem. If I understand correctly Ubuntu cloud-archive is using libvirt 1.2.12 which is *NOT* a maintenance release so that leaves us with doing an additional backport to 1.2.12 and getting the cloud-archive team to take it[1] or Adding a hack to nova. And that's just Ubuntu It's hard to say for sure that some vendor isn't running libvirt 1.2.12 also. Having said that I'm not sure which commit(s) are those that need to be back-ported. Having known your libvirt version, it shouldn't be too hard looking for the differences and finding the right commit. When back-porting request is made on the list, it is usually acted upon. If you can't find the exact commit, let me know and I'll do my best to help. So a git bisect points at: --- commit a103bb105c0c189c3973311ff1826972b5bc6ad6 Author: Daniel P. Berrange berra...@redhat.com Date: Tue Feb 10 15:59:57 2015 + qemu: fix setting of VM CPU affinity with TCG --- A small amount of reading implies to me that we'd be looking at backporting a103bb105c0c189c3973311ff1826972b5bc6ad6 to any maintenance branch that contains b07f3d821dfb11a118ee75ea275fd6ab737d9500. Which I think is 1.2.13 only, but I could be wrong. 1.2.13 has the commit already in the release and 1.2.12-maint has it as a first back-port right after release. The problem is that there was no maintenance release of 1.2.12 yet. Maybe they would use 1.2.12.1 if it existed. I Cc'd Guido as an upstream debian maintainer, maybe he'll have some insights. @Guido: would it help if we created a maintenance release from the v1.2.12-maint branch? Or is the only thing missing the fact that the launchpad bug is not moved to libvirt? Thanks guys, I'm going to cherrypick that patch into vivid right now. (It should be in wily as that is 1.2.15). Basically Ubuntu takes the version that is in Debian testing, unstable or experimental at time of their release and adds pathes. There's little to no sync unfortunately (except for the awesome efforts to sync the apparmor stuff) Debian itself is not using 1.2.12 anywhere. Current stable has 1.2.9 + maintenance patches (which I intend to switch over to the maintenance releases soonish and support hopefully Cole with these), oldstable has 0.9.12.3 and unstable/sid has 1.2.15 (and will keep following the releases). I've added Serge since he might to jump onto 1.2.12.1 maintenance relasese. If you mean contributing by adding patches that we add to vivid, I'm definately willing to do that. Note that vivid has a pretty short lifecycle so it wouldn't go on for very long. If people are willing to tag patches with '[1.2.12.1-maint]' on this list I'm willing to support this longer. I'm happy about any additional notifications for things that should go into distributions. Cheers, -- Guido Thanks! -serge -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] conf: Resolve Coverity FORWARD_NULL
Even though it's been pointed out they are false positives: http://www.redhat.com/archives/libvir-list/2015-May/msg00301.html and http://www.redhat.com/archives/libvir-list/2015-May/msg00302.html these still show up as Coverity issues. In order to silence Coverity add an 'sa_assert' prior to check failure. Signed-off-by: John Ferlan jfer...@redhat.com --- src/conf/domain_conf.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index add857c..5b69b5a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -23078,6 +23078,7 @@ virDomainObjListCollect(virDomainObjListPtr domlist, { struct virDomainListData data = { NULL, 0 }; +sa_assert(domlist-objs); virObjectLock(domlist); if (VIR_ALLOC_N(data.vms, virHashSize(domlist-objs)) 0) { virObjectUnlock(domlist); @@ -23141,6 +23142,7 @@ virDomainObjListConvert(virDomainObjListPtr domlist, } virObjectUnlock(domlist); +sa_assert(*vms); virDomainObjListFilter(vms, nvms, conn, filter, flags); return 0; -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] parallels: remove connection wide wait timeout
On 05/13/2015 05:23 PM, Nikolay Shirokovskiy wrote: We have a lot of passing arguments code just to pass connection object cause it holds jobTimeout. Taking into account that right now this value is defined at compile time let's just get rid of it and make arguments list more clear in many places. In case we later need some runtime configurable timeout value we can provide this value through arguments function already operate such as a parallels domain object etc as this timeouts are operation( and thus object) specific in practice. Signed-off-by: Nikolay Shirokovskiy nshirokovs...@parallels.com ACKed and Pushed, thanks! --- src/parallels/parallels_driver.c |9 ++-- src/parallels/parallels_sdk.c| 108 +- src/parallels/parallels_sdk.h| 27 - src/parallels/parallels_utils.h |1 - 4 files changed, 64 insertions(+), 81 deletions(-) diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c index 205ec1c..4b87213 100644 --- a/src/parallels/parallels_driver.c +++ b/src/parallels/parallels_driver.c @@ -214,7 +214,7 @@ parallelsOpenDefault(virConnectPtr conn) goto err_free; } -if (prlsdkInit(privconn)) { +if (prlsdkInit()) { VIR_DEBUG(%s, _(Can't initialize Parallels SDK)); goto err_free; } @@ -1007,7 +1007,6 @@ parallelsDomainManagedSave(virDomainPtr domain, unsigned int flags) static int parallelsDomainManagedSaveRemove(virDomainPtr domain, unsigned int flags) { -parallelsConnPtr privconn = domain-conn-privateData; virDomainObjPtr dom = NULL; int state, reason; int ret = -1; @@ -1022,7 +1021,7 @@ parallelsDomainManagedSaveRemove(virDomainPtr domain, unsigned int flags) if (!(state == VIR_DOMAIN_SHUTOFF reason == VIR_DOMAIN_SHUTOFF_SAVED)) goto cleanup; -ret = prlsdkDomainManagedSaveRemove(privconn, dom); +ret = prlsdkDomainManagedSaveRemove(dom); cleanup: virObjectUnlock(dom); @@ -1071,7 +1070,7 @@ static int parallelsDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, switch (dev-type) { case VIR_DOMAIN_DEVICE_DISK: -ret = prlsdkAttachVolume(dom-conn, privdom, dev-data.disk); +ret = prlsdkAttachVolume(privdom, dev-data.disk); if (ret) { virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(disk attach failed)); @@ -1140,7 +1139,7 @@ static int parallelsDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, switch (dev-type) { case VIR_DOMAIN_DEVICE_DISK: -ret = prlsdkDetachVolume(dom-conn, privdom, dev-data.disk); +ret = prlsdkDetachVolume(privdom, dev-data.disk); if (ret) { virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(disk detach failed)); diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c index fcd274f..d48d4b7 100644 --- a/src/parallels/parallels_sdk.c +++ b/src/parallels/parallels_sdk.c @@ -35,8 +35,6 @@ #define VIR_FROM_THIS VIR_FROM_PARALLELS #define JOB_INFINIT_WAIT_TIMEOUT UINT_MAX -PRL_UINT32 defaultJobTimeout = JOB_INFINIT_WAIT_TIMEOUT; - VIR_LOG_INIT(parallels.sdk); /* @@ -179,9 +177,9 @@ getJobResultHelper(PRL_HANDLE job, unsigned int timeout, PRL_HANDLE *result, return ret; } -#define getJobResult(job, timeout, result) \ -getJobResultHelper(job, timeout, result, __FILE__, \ - __FUNCTION__, __LINE__) +#define getJobResult(job, result) \ +getJobResultHelper(job, JOB_INFINIT_WAIT_TIMEOUT, \ +result, __FILE__, __FUNCTION__, __LINE__) static PRL_RESULT waitJobHelper(PRL_HANDLE job, unsigned int timeout, @@ -197,12 +195,13 @@ waitJobHelper(PRL_HANDLE job, unsigned int timeout, return ret; } -#define waitJob(job, timeout) \ -waitJobHelper(job, timeout, __FILE__, \ +#define waitJob(job)\ +waitJobHelper(job, JOB_INFINIT_WAIT_TIMEOUT, __FILE__, \ __FUNCTION__, __LINE__) + int -prlsdkInit(parallelsConnPtr privconn) +prlsdkInit(void) { PRL_RESULT ret; @@ -212,8 +211,6 @@ prlsdkInit(parallelsConnPtr privconn) return -1; } -privconn-jobTimeout = JOB_INFINIT_WAIT_TIMEOUT; - return 0; }; @@ -238,7 +235,7 @@ prlsdkConnect(parallelsConnPtr privconn) job = PrlSrv_LoginLocalEx(privconn-server, NULL, 0, PSL_HIGH_SECURITY, PACF_NON_INTERACTIVE_MODE); -if (waitJob(job, privconn-jobTimeout)) { +if (waitJob(job)) { PrlHandle_Free(privconn-server); return -1; } @@ -252,7 +249,7 @@ prlsdkDisconnect(parallelsConnPtr privconn) PRL_HANDLE job; job = PrlSrv_Logoff(privconn-server); -waitJob(job, privconn-jobTimeout); +
[libvirt] [PATCH 3/3] network: Resolve Coverity FORWARD_NULL
To silence Coverity just add a 'p ' in front of the check in networkFindUnusedBridgeName after the strchr() call. Even though we know it's not possible to have strchr return NULL since the only way into the function is if there is a '%' in def-bridge or it's NULL. Signed-off-by: John Ferlan jfer...@redhat.com --- src/network/bridge_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 4b53475..f438c0b 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -2780,7 +2780,7 @@ networkFindUnusedBridgeName(virNetworkObjListPtr nets, if (def-bridge (p = strchr(def-bridge, '%')) == strrchr(def-bridge, '%') -p[1] == 'd') +p p[1] == 'd') templ = def-bridge; do { -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 2/4] qemu: Implement pci-serial
On Mon, May 11, 2015 at 05:26:09PM +0200, Michal Privoznik wrote: https://bugzilla.redhat.com/show_bug.cgi?id=998813 Implementation is pretty straight-forward. Of course, not all qemus out there supports the device, so new capability is introduced and checked prior each use of the device. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c| 18 ++ tests/qemucapabilitiesdata/caps_1.3.1-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.4.2-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.5.3-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.6.0-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.6.50-1.caps | 1 + tests/qemucapabilitiesdata/caps_2.1.1-1.caps | 1 + .../qemuxml2argv-pci-serial-dev-chardev.args | 7 +++ tests/qemuxml2argvtest.c | 3 +++ 11 files changed, 37 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pci-serial-dev-chardev.args diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 25c15bf..d7bb443 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -281,6 +281,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, pc-dimm, machine-vmport-opt, /* 185 */ + pci-serial, ); @@ -1537,6 +1538,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { { iothread, QEMU_CAPS_OBJECT_IOTHREAD}, { ivshmem, QEMU_CAPS_DEVICE_IVSHMEM }, { pc-dimm, QEMU_CAPS_DEVICE_PC_DIMM }, +{ pci-serial, QEMU_CAPS_DEVICE_PCI_SERIAL }, }; static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBlk[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 81557b7..a2edf82 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -225,6 +225,7 @@ typedef enum { QEMU_CAPS_QXL_VGA_VGAMEM = 183, /* -device qxl-vga.vgamem_mb */ QEMU_CAPS_DEVICE_PC_DIMM = 184, /* pc-dimm device */ QEMU_CAPS_MACHINE_VMPORT_OPT = 185, /* -machine xxx,vmport=on/off/auto */ +QEMU_CAPS_DEVICE_PCI_SERIAL = 186, /* -device pci-serial */ QEMU_CAPS_LAST, /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index fe4622e..938dbca 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -10862,6 +10862,24 @@ qemuBuildSerialChrDeviceStr(char **deviceStr, goto error; } break; + +case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PCI: +if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PCI_SERIAL)) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, + _(pci-serial is not supported with this QEMU binary)); +goto error; +} + +if (serial-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE +serial-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, + _(pci-serial requires address of pci type)); +goto error; +} + +if (qemuBuildDeviceAddressStr(cmd, def, serial-info, qemuCaps) 0) +goto error; +break; } } You also ned to change qemuAssignDevicePCISlots(virDomainDefPtr def, virDomainPCIAddressSetPtr addrs) { for (i = 0; i def-nserials; i++) { /* Nada - none are PCI based (yet) */ } Since the comment is clearly wrong now :-) Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC v1 4/6] migration: Migration support for ephemeral hostdevs
On 05/13/2015 04:36 PM, Peter Krempa wrote: On Wed, May 13, 2015 at 11:36:30 +0800, Chen Fan wrote: add migration support for ephemeral host devices, introduce two 'detach' and 'restore' functions to unplug/plug host devices during migration. Signed-off-by: Chen Fan chen.fan.f...@cn.fujitsu.com --- src/qemu/qemu_migration.c | 171 -- src/qemu/qemu_migration.h | 9 +++ src/qemu/qemu_process.c | 11 +++ 3 files changed, 187 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 56112f9..d5a698f 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3384,6 +3384,158 @@ qemuMigrationPrepareDef(virQEMUDriverPtr driver, return def; } +int +qemuMigrationDetachEphemeralDevices(virQEMUDriverPtr driver, +virDomainObjPtr vm, +bool live) +{ +qemuDomainObjPrivatePtr priv = vm-privateData; +virDomainHostdevDefPtr hostdev; +virDomainNetDefPtr net; +virDomainDeviceDef dev; +virDomainDeviceDefPtr dev_copy = NULL; +virCapsPtr caps = NULL; +int actualType; +int ret = -1; +size_t i; + +VIR_DEBUG(Rum domain detach ephemeral devices); + +if (!(caps = virQEMUDriverGetCapabilities(driver, false))) +return ret; + +for (i = 0; i vm-def-nnets;) { +net = vm-def-nets[i]; + +actualType = virDomainNetGetActualType(net); +if (actualType != VIR_DOMAIN_NET_TYPE_HOSTDEV) { +i++; +continue; +} + +hostdev = virDomainNetGetActualHostdev(net); +if (hostdev-mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS || +hostdev-source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI || +!hostdev-ephemeral) { +i++; +continue; +} + +dev.type = VIR_DOMAIN_DEVICE_NET; +dev.data.net = net; + +dev_copy = virDomainDeviceDefCopy(dev, vm-def, + caps, driver-xmlopt); +if (!dev_copy) +goto cleanup; + +if (live) { +/* nnets reduced */ +if (qemuDomainDetachNetDevice(driver, vm, dev_copy) 0) +goto cleanup; So this is where the fun begins. qemuDomainDetachNetDevice is not designed to be called this way since the detach API where it's used normally returns 0 in the following two cases: 1) The detach was successfull, the guest removed the device 2) The detach request was successful, but guest did not remove the device yet In the latter case you need to wait for a event to successfully know when the device was removed. Since this might very well happen the code will need to be changed to take that option into account. Please note that that step will make all the things really complicated. did you said the event is DEVICE_DELETED ? I saw the code the funcition qemuDomainWaitForDeviceRemoval has been used for waiting device removed from guest. Thanks, Chen Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: clear useless code
The variable 'now' is useless in qemuMigrationPrepareAny(), so I clear it. Signed-off-by: Wang Yufei james.wangyu...@huawei.com Signed-off-by: Zhang Bo oscar.zhan...@huawei.com --- src/qemu/qemu_migration.c | 4 1 file changed, 4 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index c1af704..4dcba7a 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2913,7 +2913,6 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, int ret = -1; int dataFD[2] = { -1, -1 }; qemuDomainObjPrivatePtr priv = NULL; -unsigned long long now; qemuMigrationCookiePtr mig = NULL; bool tunnel = !!st; char *xmlout = NULL; @@ -2923,9 +2922,6 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, bool abort_on_error = !!(flags VIR_MIGRATE_ABORT_ON_ERROR); bool taint_hook = false; -if (virTimeMillisNow(now) 0) -return -1; - virNWFilterReadLockFilterUpdates(); if (flags VIR_MIGRATE_OFFLINE) { -- 1.7.12.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Taint domains using cdrom-passthrough
On Tue, May 12, 2015 at 16:03:33 -0400, John Ferlan wrote: https://bugzilla.redhat.com/show_bug.cgi?id=976387 For a domain configured using the host cdrom, we should taint the domain due to problems encountered when the host and guest try to control the tray. Signed-off-by: John Ferlan jfer...@redhat.com --- src/conf/domain_conf.c | 3 ++- src/conf/domain_conf.h | 1 + src/qemu/qemu_domain.c | 6 ++ 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index add857c..a67e200 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -101,7 +101,8 @@ VIR_ENUM_IMPL(virDomainTaint, VIR_DOMAIN_TAINT_LAST, disk-probing, external-launch, host-cpu, - hook-script); + hook-script, + cdrom-passthrough); VIR_ENUM_IMPL(virDomainVirt, VIR_DOMAIN_VIRT_LAST, qemu, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 2cd105a7..0867e8b 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2280,6 +2280,7 @@ typedef enum { VIR_DOMAIN_TAINT_EXTERNAL_LAUNCH, /* Externally launched guest domain */ VIR_DOMAIN_TAINT_HOST_CPU, /* Host CPU passthrough in use */ VIR_DOMAIN_TAINT_HOOK, /* Domain (possibly) changed via hook script */ +VIR_DOMAIN_TAINT_CDROM_PASSTHROUGH,/* CDROM passthrough */ VIR_DOMAIN_TAINT_LAST } virDomainTaintFlags; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index fa8229f..b66ee89 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2031,6 +2031,12 @@ void qemuDomainObjCheckDiskTaint(virQEMUDriverPtr driver, qemuDomainObjTaint(driver, obj, VIR_DOMAIN_TAINT_HIGH_PRIVILEGES, logFD); +if (disk-device == VIR_DOMAIN_DISK_DEVICE_CDROM +virStorageSourceGetActualType(disk-src) == VIR_STORAGE_TYPE_BLOCK +disk-src-path) +qemuDomainObjTaint(driver, obj, VIR_DOMAIN_TAINT_CDROM_PASSTHROUGH, + logFD); + This won't be enough currently since you can change the media in the CDROM so that it becomes a passthrough device later in it's lifecycle. You'll need to call qemuDomainObjCheckDiskTaint in qemuDomainUpdateDeviceLive too once you'll be using it to mark those. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Xen-devel] [PATCH] libxl: Disallow save or migrate when host devices are assigned to a guest.
On Wed, 2015-05-06 at 11:59 -0600, Jim Fehlig wrote: Dario Faggioli wrote: On Fri, 2015-05-01 at 15:32 -0600, Jim Fehlig wrote: E.g. if a domain is configured to use memory allocated from certain numa nodes, we'd want to make sure the dest has sufficient memory amongst those nodes. But these were just examples of things we may want to check before starting the migration. I suppose there are better examples wrt storage, e.g. does the domain have an attached DVD, ... Do (if you know that) other drivers in libvirt do these kind of checks? Yes. The qemu driver has quite extensive migration checks. Interesting. I never had the chance to check that. I think we should have a look and try to figure out whether some o them make sense for us too, and decide whether we want them in libxl or in libvirt. I'm adding this to my TODO list. Thanks and Regards, Dario signature.asc Description: This is a digitally signed message part -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: vnc: error out for invalid port number
On Tue, May 12, 2015 at 07:00:07PM +0200, Pavel Hrdina wrote: In the XML we have the vnc port number, but QEMU takes on command line a vnc screen number, it's port-5900. We should fail with error message that only ports in range [5900,65535] are valid. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1164966 Signed-off-by: Pavel Hrdina phrd...@redhat.com --- src/qemu/qemu_command.c | 96 +++-- 1 file changed, 53 insertions(+), 43 deletions(-) What a weird diff, have you used --patience for formatting? It might be worth putting a note here that the diff is best viewed using '-w' or even better '-w --word-diff' so that reviewer notices immediatelly that most of the change is just whitespace movement. ACK. signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] sysinfo: Fix reports on arm
On Tue, May 12, 2015 at 11:44:49AM -0600, Eric Blake wrote: On 05/12/2015 10:23 AM, Michal Privoznik wrote: Due to a commit in kernel (155597223) it's 'processor' rather than 'Processor'. Fix our parser too. Shouldn't we be parsing this string case-insensitively, to work with kernels that pre-date that kernel commit? The referenced commit is from 2005. I would not lose sleep over libvirt not working with a ten year old kernel. But the libvirt code using Processor was added by commit 347081e [1] in 2013. Looks like the relevant kernel commit here is b4b8f770e [2] from 2012. After that the model name is printed under 'model name', not 'Processor:', so I'd expect your patch to break the test case added by [1]. Adding a new test case is very welcome. Jan [1] http://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=347081e [2] https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=b4b8f770e Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/util/virsysinfo.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/virsysinfo.c b/src/util/virsysinfo.c index 8bb17f0..fb8cb2c 100644 --- a/src/util/virsysinfo.c +++ b/src/util/virsysinfo.c @@ -289,7 +289,7 @@ virSysinfoParseProcessor(const char *base, virSysinfoDefPtr ret) virSysinfoProcessorDefPtr processor; char *processor_type = NULL; -if (!(tmp_base = strstr(base, Processor))) +if (!(tmp_base = strstr(base, processor))) return 0; base = tmp_base; signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC v1 0/6] Live Migration with ephemeral host NIC devices
On Wed, May 13, 2015 at 11:36:26 +0800, Chen Fan wrote: my main goal is to add support migration with host NIC passthrough devices and keep the network connectivity. this series patch base on Shradha's patches on https://www.redhat.com/archives/libvir-list/2012-November/msg01324.html which is add migration support for host passthrough devices. 1) unplug the ephemeral devices before migration 2) do native migration 3) when migration finished, hotplug the ephemeral devices IMHO this algorithm is something that an upper layer management app should do. The device unplug operation is complex and it might not succeed which will make the current migration thread hang or fail in an intermediate state that will not be recoverable. I'll point the places out in the actual patches. TODO: keep network connectivity on guest level by bonding device. This is out of scope for libvirt since we don't really know what is running inside the vm. Chen Fan (6): conf: add ephemeral element for hostdev supporting migration qemu: Save ephemeral devices into qemuDomainObjPrivate qemu: add check ephemeral devices only for PCI host devices migration: Migration support for ephemeral hostdevs managedsave: move the domain xml handling forward to stop CPU managedsave: add managedsave support for ephemeral host devices Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC v1 0/6] Live Migration with ephemeral host NIC devices
* Peter Krempa (pkre...@redhat.com) wrote: On Wed, May 13, 2015 at 11:36:26 +0800, Chen Fan wrote: my main goal is to add support migration with host NIC passthrough devices and keep the network connectivity. this series patch base on Shradha's patches on https://www.redhat.com/archives/libvir-list/2012-November/msg01324.html which is add migration support for host passthrough devices. 1) unplug the ephemeral devices before migration 2) do native migration 3) when migration finished, hotplug the ephemeral devices IMHO this algorithm is something that an upper layer management app should do. The device unplug operation is complex and it might not succeed which will make the current migration thread hang or fail in an intermediate state that will not be recoverable. However you wouldn't want each of the upper layer management apps implementing their own hacks for this; so something somewhere needs to standardise what the guest sees. Dave I'll point the places out in the actual patches. TODO: keep network connectivity on guest level by bonding device. This is out of scope for libvirt since we don't really know what is running inside the vm. Chen Fan (6): conf: add ephemeral element for hostdev supporting migration qemu: Save ephemeral devices into qemuDomainObjPrivate qemu: add check ephemeral devices only for PCI host devices migration: Migration support for ephemeral hostdevs managedsave: move the domain xml handling forward to stop CPU managedsave: add managedsave support for ephemeral host devices Peter -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC v1 0/6] Live Migration with ephemeral host NIC devices
On Wed, May 13, 2015 at 10:05:24AM +0200, Peter Krempa wrote: On Wed, May 13, 2015 at 11:36:26 +0800, Chen Fan wrote: my main goal is to add support migration with host NIC passthrough devices and keep the network connectivity. this series patch base on Shradha's patches on https://www.redhat.com/archives/libvir-list/2012-November/msg01324.html which is add migration support for host passthrough devices. 1) unplug the ephemeral devices before migration 2) do native migration 3) when migration finished, hotplug the ephemeral devices IMHO this algorithm is something that an upper layer management app should do. The device unplug operation is complex and it might not succeed which will make the current migration thread hang or fail in an intermediate state that will not be recoverable. Agreed, that's what I have said in response to this suggestion many times before. This kind of thing really falls into the realm of usage policy, and we've long said that libvirt should focus on providing the /mechanism/ and leave usage policy upto the management application. There are many possible policies, and libvirt should not be trying to decide which is best for all applications. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC v1 3/6] qemu: add check ephemeral devices only for PCI host devices
On Wed, May 13, 2015 at 11:36:29 +0800, Chen Fan wrote: currently, we only support PCI host devices with ephemeral flag. and USB already supports migration. so maybe in the near future we can support SCSI. Signed-off-by: Chen Fan chen.fan.f...@cn.fujitsu.com --- src/qemu/qemu_command.c | 10 ++ src/qemu/qemu_migration.c | 11 +++ 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index fc81214..5acd8b4 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -10182,6 +10182,16 @@ qemuBuildCommandLine(virConnectPtr conn, /* PCI */ if (hostdev-mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS +hostdev-source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB +(hostdev-source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI + hostdev-ephemeral)) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, + _(non-USB and non-PCI device assignment with ephemeral + flag are not supported by this version of qemu)); This functionality is not based on qemu support but on libvirt implementation so the error message is incorrect. +goto error; +} + +if (hostdev-mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS hostdev-source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) { int backend = hostdev-source.subsys.u.pci.backend; diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 83be435..56112f9 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1981,21 +1981,24 @@ qemuMigrationIsAllowed(virQEMUDriverPtr driver, virDomainObjPtr vm, def = vm-def; } -/* Migration with USB host devices is allowed, all other devices are - * forbidden. +/* + * Migration with USB and ephemeral PCI host devices host devices are allowed, + * all other devices are forbidden. */ forbid = false; for (i = 0; i def-nhostdevs; i++) { virDomainHostdevDefPtr hostdev = def-hostdevs[i]; if (hostdev-mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS || -hostdev-source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) { +(hostdev-source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB +!hostdev-ephemeral)) { forbid = true; break; } } if (forbid) { virReportError(VIR_ERR_OPERATION_INVALID, %s, - _(domain has assigned non-USB host devices)); + _(domain has assigned non-USB and + non-ephemeral host devices)); return false; } This patch has to be moved after you actually implement the ephemeral device unplug code, since an intermediate state would allow to bypass the check while the devices actually would not be unplugged. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] time: event poll may become un-triggerable after changing system clock.
On Wed, May 13, 2015 at 08:54:49AM +0800, zhang bo wrote: On 2015/5/12 16:56, Daniel P. Berrange wrote: On Tue, May 12, 2015 at 04:14:37PM +0800, zhang bo wrote: So, Is there any other better solution? thanks in advance. Simply don't change the system time by massive deltas. Libvirt is not going to be the only app to be affected. As you mention it is going to hit the pthread_cond_wait() call which will likely affect pretty much every single non-trivial process running on the system. I'd expect other apps have much the same problem with calculating poll sleeps too. If you need to massively change the system time this should be done at single user mode, or do a reboot. Once a system is running it should be kept synced with NTPD which will only ever change system time in very small increments and so once cause thsi problem. Would you please explain more about what single user mode mean here? and does reboot refers to rebooting the os or rebooting the process libvirtd? I think it's the process, right? Single user mode is where you boot the OS such that you only have a root shell running. No other processes are present. So you can safely mess around with core system settings like hardware clock without confusing any apps that are running. I mean rebooting the OS, not restarting the process Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: vnc: error out for invalid port number
On Wed, May 13, 2015 at 09:51:08AM +0200, Martin Kletzander wrote: On Tue, May 12, 2015 at 07:00:07PM +0200, Pavel Hrdina wrote: In the XML we have the vnc port number, but QEMU takes on command line a vnc screen number, it's port-5900. We should fail with error message that only ports in range [5900,65535] are valid. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1164966 Signed-off-by: Pavel Hrdina phrd...@redhat.com --- src/qemu/qemu_command.c | 96 +++-- 1 file changed, 53 insertions(+), 43 deletions(-) What a weird diff, have you used --patience for formatting? It might be worth putting a note here that the diff is best viewed using '-w' or even better '-w --word-diff' so that reviewer notices immediatelly that most of the change is just whitespace movement. ACK. Thanks for review. Yes, I've used --patience for formatting and I would mention the -w or --word-diff options, if I knew them. I'll push it shortly. Pavel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list