Re: [libvirt] [PATCHv2] Ignore bridge template names with multiple printf conversions

2015-05-13 Thread Ján Tomko
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

2015-05-13 Thread Dr. David Alan Gilbert
* 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

2015-05-13 Thread Daniel P. Berrange
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

2015-05-13 Thread Dr. David Alan Gilbert
* 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

2015-05-13 Thread Daniel P. Berrange
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.

2015-05-13 Thread Andrea Bolognani
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

2015-05-13 Thread Peter Krempa
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

2015-05-13 Thread Daniel P. Berrange
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

2015-05-13 Thread Daniel P. Berrange
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

2015-05-13 Thread Dr. David Alan Gilbert
* 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

2015-05-13 Thread John Ferlan


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

2015-05-13 Thread Peter Krempa
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

2015-05-13 Thread Peter Krempa
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

2015-05-13 Thread Daniel P. Berrange
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

2015-05-13 Thread Jiri Denemark
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

2015-05-13 Thread Dmitry Guryanov

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

2015-05-13 Thread Martin Kletzander
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

2015-05-13 Thread Martin Kletzander
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

2015-05-13 Thread Martin Kletzander
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

2015-05-13 Thread Jiri Denemark
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

2015-05-13 Thread Martin Kletzander
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

2015-05-13 Thread Michal Privoznik
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

2015-05-13 Thread John Ferlan


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

2015-05-13 Thread Michal Privoznik
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

2015-05-13 Thread Martin Kletzander
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

2015-05-13 Thread Jiri Denemark
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

2015-05-13 Thread Jiri Denemark
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

2015-05-13 Thread Jiri Denemark
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

2015-05-13 Thread Jiri Denemark
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

2015-05-13 Thread Laine Stump
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

2015-05-13 Thread Nikolay Shirokovskiy
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.

2015-05-13 Thread Eric Blake
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

2015-05-13 Thread Laine Stump
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

2015-05-13 Thread Eric Blake
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

2015-05-13 Thread Dr. David Alan Gilbert
* 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

2015-05-13 Thread Michal Privoznik
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.

2015-05-13 Thread Eric Blake
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

2015-05-13 Thread John Ferlan


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

2015-05-13 Thread Laine Stump
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

2015-05-13 Thread Laine Stump
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

2015-05-13 Thread Jiri Denemark
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

2015-05-13 Thread Dr. David Alan Gilbert
* 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

2015-05-13 Thread John Ferlan
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

2015-05-13 Thread John Ferlan
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

2015-05-13 Thread Chen Fan


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

2015-05-13 Thread Chen Fan


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

2015-05-13 Thread Serge Hallyn
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

2015-05-13 Thread John Ferlan
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

2015-05-13 Thread Dmitry Guryanov

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

2015-05-13 Thread John Ferlan
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

2015-05-13 Thread Daniel P. Berrange
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

2015-05-13 Thread Chen Fan


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

2015-05-13 Thread zhang bo
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

2015-05-13 Thread Peter Krempa
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.

2015-05-13 Thread Dario Faggioli
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

2015-05-13 Thread Martin Kletzander

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

2015-05-13 Thread Ján Tomko
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

2015-05-13 Thread Peter Krempa
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

2015-05-13 Thread Dr. David Alan Gilbert
* 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

2015-05-13 Thread Daniel P. Berrange
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

2015-05-13 Thread Peter Krempa
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.

2015-05-13 Thread Daniel P. Berrange
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

2015-05-13 Thread Pavel Hrdina
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