Re: [libvirt] [PATCH 16/34] Clear assigned PCI devices at shutdown

2010-01-15 Thread Daniel Veillard
On Fri, Jan 08, 2010 at 05:23:12PM +, Daniel P. Berrange wrote:
> The PCI device addresses are only valid while the VM is running,
> since they are auto-assigned by QEMU. After shutdown they must
> all be cleared. Future QEMU driver enhancement will allow for
> persistent PCI address assignment
> 
> * src/conf/domain_conf.h, src/conf/domain_conf.c, src/libvirt_private.syms
>   Add virDomainDefClearPCIAddresses() method for wiping out auto assigned
>   PCI addresses
> * src/qemu/qemu_driver.c: Clear PCI addresses at VM shutdown
> ---
>  src/conf/domain_conf.c   |   36 
>  src/conf/domain_conf.h   |2 ++
>  src/libvirt_private.syms |1 +
>  src/qemu/qemu_driver.c   |2 ++
>  4 files changed, 41 insertions(+), 0 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 5edd060..ef5dbe9 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -816,6 +816,42 @@ void virDomainDeviceInfoClear(virDomainDeviceInfoPtr 
> info)
>  }
>  
>  
> +static void virDomainDeviceInfoClearField(virDomainDeviceInfoPtr info)
> +{
> +if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
> +memset(&info->addr, 0, sizeof(info->addr));
> +info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE;
> +}
> +}
> +
> +
> +static void virDomainDefClearDeviceInfo(virDomainDefPtr def)
> +{
> +int i;
> +
> +for (i = 0; i < def->ndisks ; i++)
> +virDomainDeviceInfoClearField(&def->disks[i]->info);
> +for (i = 0; i < def->nnets ; i++)
> +virDomainDeviceInfoClearField(&def->nets[i]->info);
> +for (i = 0; i < def->nsounds ; i++)
> +virDomainDeviceInfoClearField(&def->sounds[i]->info);
> +for (i = 0; i < def->nhostdevs ; i++)
> +virDomainDeviceInfoClearField(&def->hostdevs[i]->info);
> +for (i = 0; i < def->nvideos ; i++)
> +virDomainDeviceInfoClearField(&def->videos[i]->info);
> +for (i = 0; i < def->ncontrollers ; i++)
> +virDomainDeviceInfoClearField(&def->controllers[i]->info);
> +if (def->watchdog)
> +virDomainDeviceInfoClearField(&def->watchdog->info);
> +}
> +
> +
> +void virDomainDefClearPCIAddresses(virDomainDefPtr def)
> +{
> +virDomainDefClearDeviceInfo(def);
> +}
> +
> +
>  /* Generate a string representation of a device address
>   * @param address Device address to stringify
>   */
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 1831d17..a6f7ab2 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -736,6 +736,8 @@ int 
> virDomainDevicePCIAddressIsValid(virDomainDevicePCIAddressPtr addr);
>  int virDomainDeviceDriveAddressIsValid(virDomainDeviceDriveAddressPtr addr);
>  int virDomainDeviceInfoIsSet(virDomainDeviceInfoPtr info);
>  void virDomainDeviceInfoClear(virDomainDeviceInfoPtr info);
> +void virDomainDefClearPCIAddresses(virDomainDefPtr def);
> +
>  void virDomainDefFree(virDomainDefPtr vm);
>  void virDomainObjRef(virDomainObjPtr vm);
>  /* Returns 1 if the object was freed, 0 if more refs exist */
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index a4a02e7..678d610 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -187,6 +187,7 @@ virDomainControllerTypeToString;
>  virDomainControllerDefFree;
>  virDomainDeviceAddressTypeToString;
>  virDomainDefAddDiskControllers;
> +virDomainDefClearPCIAddresses;
>  
>  
>  # domain_event.h
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index cd406ec..6a3af61 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -3032,6 +3032,8 @@ static void qemudShutdownVMDaemon(virConnectPtr conn,
>  VIR_FREE(vm->def->seclabel.imagelabel);
>  }
>  
> +virDomainDefClearPCIAddresses(vm->def);
> +
>  if (qemuDomainSetAllDeviceOwnership(conn, driver, vm->def, 1) < 0)
>  VIR_WARN("Failed to restore all device ownership for %s",
>   vm->def->name);
> -- 
> 1.6.5.2
> 

  Hum, I don't see in the patch what's makes the difference between
an auto-assigned PCI address and one which might be set by the user.
Since it shows up in the XML construct it has to be setable by the user
so I'm a bit confused here,

  ACK,

Daniel

-- 
Daniel Veillard  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
dan...@veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH 16/34] Clear assigned PCI devices at shutdown

2010-01-15 Thread Daniel P. Berrange
On Fri, Jan 15, 2010 at 05:12:52PM +0100, Daniel Veillard wrote:
> On Fri, Jan 08, 2010 at 05:23:12PM +, Daniel P. Berrange wrote:
> > The PCI device addresses are only valid while the VM is running,
> > since they are auto-assigned by QEMU. After shutdown they must
> > all be cleared. Future QEMU driver enhancement will allow for
> > persistent PCI address assignment
> > 
> > * src/conf/domain_conf.h, src/conf/domain_conf.c, src/libvirt_private.syms
> >   Add virDomainDefClearPCIAddresses() method for wiping out auto assigned
> >   PCI addresses
> > * src/qemu/qemu_driver.c: Clear PCI addresses at VM shutdown
> > ---
> >  src/conf/domain_conf.c   |   36 
> >  src/conf/domain_conf.h   |2 ++
> >  src/libvirt_private.syms |1 +
> >  src/qemu/qemu_driver.c   |2 ++
> >  4 files changed, 41 insertions(+), 0 deletions(-)
> > 
> > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> > index 5edd060..ef5dbe9 100644
> > --- a/src/conf/domain_conf.c
> > +++ b/src/conf/domain_conf.c
> > @@ -816,6 +816,42 @@ void virDomainDeviceInfoClear(virDomainDeviceInfoPtr 
> > info)
> >  }
> >  
> >  
> > +static void virDomainDeviceInfoClearField(virDomainDeviceInfoPtr info)
> > +{
> > +if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
> > +memset(&info->addr, 0, sizeof(info->addr));
> > +info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE;
> > +}
> > +}
> > +
> > +
> > +static void virDomainDefClearDeviceInfo(virDomainDefPtr def)
> > +{
> > +int i;
> > +
> > +for (i = 0; i < def->ndisks ; i++)
> > +virDomainDeviceInfoClearField(&def->disks[i]->info);
> > +for (i = 0; i < def->nnets ; i++)
> > +virDomainDeviceInfoClearField(&def->nets[i]->info);
> > +for (i = 0; i < def->nsounds ; i++)
> > +virDomainDeviceInfoClearField(&def->sounds[i]->info);
> > +for (i = 0; i < def->nhostdevs ; i++)
> > +virDomainDeviceInfoClearField(&def->hostdevs[i]->info);
> > +for (i = 0; i < def->nvideos ; i++)
> > +virDomainDeviceInfoClearField(&def->videos[i]->info);
> > +for (i = 0; i < def->ncontrollers ; i++)
> > +virDomainDeviceInfoClearField(&def->controllers[i]->info);
> > +if (def->watchdog)
> > +virDomainDeviceInfoClearField(&def->watchdog->info);
> > +}
> > +
> > +
> > +void virDomainDefClearPCIAddresses(virDomainDefPtr def)
> > +{
> > +virDomainDefClearDeviceInfo(def);
> > +}
> > +
> > +
> >  /* Generate a string representation of a device address
> >   * @param address Device address to stringify
> >   */
> > diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> > index 1831d17..a6f7ab2 100644
> > --- a/src/conf/domain_conf.h
> > +++ b/src/conf/domain_conf.h
> > @@ -736,6 +736,8 @@ int 
> > virDomainDevicePCIAddressIsValid(virDomainDevicePCIAddressPtr addr);
> >  int virDomainDeviceDriveAddressIsValid(virDomainDeviceDriveAddressPtr 
> > addr);
> >  int virDomainDeviceInfoIsSet(virDomainDeviceInfoPtr info);
> >  void virDomainDeviceInfoClear(virDomainDeviceInfoPtr info);
> > +void virDomainDefClearPCIAddresses(virDomainDefPtr def);
> > +
> >  void virDomainDefFree(virDomainDefPtr vm);
> >  void virDomainObjRef(virDomainObjPtr vm);
> >  /* Returns 1 if the object was freed, 0 if more refs exist */
> > diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> > index a4a02e7..678d610 100644
> > --- a/src/libvirt_private.syms
> > +++ b/src/libvirt_private.syms
> > @@ -187,6 +187,7 @@ virDomainControllerTypeToString;
> >  virDomainControllerDefFree;
> >  virDomainDeviceAddressTypeToString;
> >  virDomainDefAddDiskControllers;
> > +virDomainDefClearPCIAddresses;
> >  
> >  
> >  # domain_event.h
> > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> > index cd406ec..6a3af61 100644
> > --- a/src/qemu/qemu_driver.c
> > +++ b/src/qemu/qemu_driver.c
> > @@ -3032,6 +3032,8 @@ static void qemudShutdownVMDaemon(virConnectPtr conn,
> >  VIR_FREE(vm->def->seclabel.imagelabel);
> >  }
> >  
> > +virDomainDefClearPCIAddresses(vm->def);
> > +
> >  if (qemuDomainSetAllDeviceOwnership(conn, driver, vm->def, 1) < 0)
> >  VIR_WARN("Failed to restore all device ownership for %s",
> >   vm->def->name);
> > -- 
> > 1.6.5.2
> > 
> 
>   Hum, I don't see in the patch what's makes the difference between
> an auto-assigned PCI address and one which might be set by the user.
> Since it shows up in the XML construct it has to be setable by the user
> so I'm a bit confused here,

This patch series is currently not supporting persistent addresses, so
all addresses will be cleared.

A later patch will allow persistence when using QEMU >= 0.12  (it isn't
possible for older QEMU at all)

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danb

[libvirt] [PATCH 16/34] Clear assigned PCI devices at shutdown

2010-01-08 Thread Daniel P. Berrange
The PCI device addresses are only valid while the VM is running,
since they are auto-assigned by QEMU. After shutdown they must
all be cleared. Future QEMU driver enhancement will allow for
persistent PCI address assignment

* src/conf/domain_conf.h, src/conf/domain_conf.c, src/libvirt_private.syms
  Add virDomainDefClearPCIAddresses() method for wiping out auto assigned
  PCI addresses
* src/qemu/qemu_driver.c: Clear PCI addresses at VM shutdown
---
 src/conf/domain_conf.c   |   36 
 src/conf/domain_conf.h   |2 ++
 src/libvirt_private.syms |1 +
 src/qemu/qemu_driver.c   |2 ++
 4 files changed, 41 insertions(+), 0 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 5edd060..ef5dbe9 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -816,6 +816,42 @@ void virDomainDeviceInfoClear(virDomainDeviceInfoPtr info)
 }
 
 
+static void virDomainDeviceInfoClearField(virDomainDeviceInfoPtr info)
+{
+if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
+memset(&info->addr, 0, sizeof(info->addr));
+info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE;
+}
+}
+
+
+static void virDomainDefClearDeviceInfo(virDomainDefPtr def)
+{
+int i;
+
+for (i = 0; i < def->ndisks ; i++)
+virDomainDeviceInfoClearField(&def->disks[i]->info);
+for (i = 0; i < def->nnets ; i++)
+virDomainDeviceInfoClearField(&def->nets[i]->info);
+for (i = 0; i < def->nsounds ; i++)
+virDomainDeviceInfoClearField(&def->sounds[i]->info);
+for (i = 0; i < def->nhostdevs ; i++)
+virDomainDeviceInfoClearField(&def->hostdevs[i]->info);
+for (i = 0; i < def->nvideos ; i++)
+virDomainDeviceInfoClearField(&def->videos[i]->info);
+for (i = 0; i < def->ncontrollers ; i++)
+virDomainDeviceInfoClearField(&def->controllers[i]->info);
+if (def->watchdog)
+virDomainDeviceInfoClearField(&def->watchdog->info);
+}
+
+
+void virDomainDefClearPCIAddresses(virDomainDefPtr def)
+{
+virDomainDefClearDeviceInfo(def);
+}
+
+
 /* Generate a string representation of a device address
  * @param address Device address to stringify
  */
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 1831d17..a6f7ab2 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -736,6 +736,8 @@ int 
virDomainDevicePCIAddressIsValid(virDomainDevicePCIAddressPtr addr);
 int virDomainDeviceDriveAddressIsValid(virDomainDeviceDriveAddressPtr addr);
 int virDomainDeviceInfoIsSet(virDomainDeviceInfoPtr info);
 void virDomainDeviceInfoClear(virDomainDeviceInfoPtr info);
+void virDomainDefClearPCIAddresses(virDomainDefPtr def);
+
 void virDomainDefFree(virDomainDefPtr vm);
 void virDomainObjRef(virDomainObjPtr vm);
 /* Returns 1 if the object was freed, 0 if more refs exist */
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index a4a02e7..678d610 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -187,6 +187,7 @@ virDomainControllerTypeToString;
 virDomainControllerDefFree;
 virDomainDeviceAddressTypeToString;
 virDomainDefAddDiskControllers;
+virDomainDefClearPCIAddresses;
 
 
 # domain_event.h
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index cd406ec..6a3af61 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3032,6 +3032,8 @@ static void qemudShutdownVMDaemon(virConnectPtr conn,
 VIR_FREE(vm->def->seclabel.imagelabel);
 }
 
+virDomainDefClearPCIAddresses(vm->def);
+
 if (qemuDomainSetAllDeviceOwnership(conn, driver, vm->def, 1) < 0)
 VIR_WARN("Failed to restore all device ownership for %s",
  vm->def->name);
-- 
1.6.5.2

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list