1. Patchew
  2. Libvirt
  3. View series

[PATCH] test_driver: support VIR_DOMAIN_AFFECT_LIVE in testUpdateDeviceFlags()

John Levon posted 1 patch 2 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20240704125946.474261-1-john.levon@nutanix.com
src/test/test_driver.c | 54 +++++++++++++++++++++++++++++-------------
1 file changed, 37 insertions(+), 17 deletions(-)
[PATCH] test_driver: support VIR_DOMAIN_AFFECT_LIVE in testUpdateDeviceFlags()
Posted by John Levon 2 months ago 
Pick up some more of the qemu_driver.c code so this function supports
both CONFIG and LIVE updates.

Note that qemuDomainUpdateDeviceFlags() passed vm->def to
virDomainDeviceDefParse() for the VIR_DOMAIN_AFFECT_CONFIG case, which
is technically incorrect; in the test driver code we'll fix this.

Signed-off-by: John Levon <john.levon@nutanix.com>
---
 src/test/test_driver.c | 54 +++++++++++++++++++++++++++++-------------
 1 file changed, 37 insertions(+), 17 deletions(-)

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 712bb20563..da682da9ad 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -10239,10 +10239,10 @@ testDomainAttachDevice(virDomainPtr domain, const char *xml)
 
 
 static int
-testDomainUpdateDeviceConfig(virDomainDef *vmdef,
-                             virDomainDeviceDef *dev,
-                             unsigned int parse_flags,
-                             virDomainXMLOption *xmlopt)
+testDomainUpdateDevice(virDomainDef *vmdef,
+                       virDomainDeviceDef *dev,
+                       unsigned int parse_flags,
+                       virDomainXMLOption *xmlopt)
 {
     virDomainDiskDef *newDisk;
     virDomainDeviceDef oldDev = { .type = dev->type };
@@ -10316,12 +10316,16 @@ testDomainUpdateDeviceFlags(virDomainPtr dom,
     testDriver *driver = dom->conn->privateData;
     virDomainObj *vm = NULL;
     virObjectEvent *event = NULL;
+    virDomainDef *def = NULL;
+    virDomainDef *persistentDef = NULL;
     g_autoptr(virDomainDef) vmdef = NULL;
-    g_autoptr(virDomainDeviceDef) dev = NULL;
+    g_autoptr(virDomainDeviceDef) dev_live = NULL;
+    g_autoptr(virDomainDeviceDef) dev_config = NULL;
     int ret = -1;
     unsigned int parse_flags = 0;
 
-    virCheckFlags(VIR_DOMAIN_AFFECT_CONFIG, -1);
+    virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
+                  VIR_DOMAIN_AFFECT_CONFIG, -1);
 
     if (!(vm = testDomObjFromDomain(dom)))
         goto cleanup;
@@ -10337,9 +10341,24 @@ testDomainUpdateDeviceFlags(virDomainPtr dom,
         parse_flags |= VIR_DOMAIN_DEF_PARSE_INACTIVE;
     }
 
-    if (!(dev = virDomainDeviceDefParse(xml, vm->def, driver->xmlopt,
-                                        NULL, parse_flags))) {
-        goto endjob;
+    /*
+     * NB: this has diverged from qemuDomainUpdateDeviceFlags(), which uses
+     * vm->def in both cases; technically the qemu driver should do the same.
+     */
+    if (virDomainObjGetDefs(vm, flags, &def, &persistentDef) < 0)
+        goto cleanup;
+
+    if (def) {
+        if (!(dev_live = virDomainDeviceDefParse(xml, def, driver->xmlopt,
+                                                 NULL, parse_flags)))
+            goto endjob;
+    }
+
+    if (persistentDef) {
+        if (!(dev_config = virDomainDeviceDefParse(xml, persistentDef,
+                                                   driver->xmlopt, NULL,
+                                                   parse_flags)))
+            goto endjob;
     }
 
     if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
@@ -10350,18 +10369,19 @@ testDomainUpdateDeviceFlags(virDomainPtr dom,
 
         /* virDomainDefCompatibleDevice call is delayed until we know the
          * device we're going to update. */
-        if ((ret = testDomainUpdateDeviceConfig(vmdef, dev,
-                                                parse_flags,
-                                                driver->xmlopt)) < 0)
+        if ((ret = testDomainUpdateDevice(vmdef, dev_config,
+                                          parse_flags,
+                                          driver->xmlopt)) < 0)
             goto endjob;
     }
 
     if (flags & VIR_DOMAIN_AFFECT_LIVE) {
-        ret = -1;
-        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                       _("live update of device '%1$s' is not supported"),
-                       virDomainDeviceTypeToString(dev->type));
-        goto endjob;
+        /* virDomainDefCompatibleDevice call is delayed until we know the
+         * device we're going to update. */
+        if ((ret = testDomainUpdateDevice(def, dev_live,
+                                          parse_flags,
+                                          driver->xmlopt)) < 0)
+            goto endjob;
     }
 
     if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
-- 
2.34.1
Re: [PATCH] test_driver: support VIR_DOMAIN_AFFECT_LIVE in testUpdateDeviceFlags()
Posted by Michal Prívozník 2 months ago 
On 7/4/24 14:59, John Levon wrote:
> Pick up some more of the qemu_driver.c code so this function supports
> both CONFIG and LIVE updates.
> 
> Note that qemuDomainUpdateDeviceFlags() passed vm->def to
> virDomainDeviceDefParse() for the VIR_DOMAIN_AFFECT_CONFIG case, which
> is technically incorrect; in the test driver code we'll fix this.
> 
> Signed-off-by: John Levon <john.levon@nutanix.com>
> ---
>  src/test/test_driver.c | 54 +++++++++++++++++++++++++++++-------------
>  1 file changed, 37 insertions(+), 17 deletions(-)
> 
> diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> index 712bb20563..da682da9ad 100644
> --- a/src/test/test_driver.c
> +++ b/src/test/test_driver.c
> @@ -10239,10 +10239,10 @@ testDomainAttachDevice(virDomainPtr domain, const char *xml)
>  
>  
>  static int
> -testDomainUpdateDeviceConfig(virDomainDef *vmdef,
> -                             virDomainDeviceDef *dev,
> -                             unsigned int parse_flags,
> -                             virDomainXMLOption *xmlopt)
> +testDomainUpdateDevice(virDomainDef *vmdef,
> +                       virDomainDeviceDef *dev,
> +                       unsigned int parse_flags,
> +                       virDomainXMLOption *xmlopt)
>  {
>      virDomainDiskDef *newDisk;
>      virDomainDeviceDef oldDev = { .type = dev->type };
> @@ -10316,12 +10316,16 @@ testDomainUpdateDeviceFlags(virDomainPtr dom,
>      testDriver *driver = dom->conn->privateData;
>      virDomainObj *vm = NULL;
>      virObjectEvent *event = NULL;
> +    virDomainDef *def = NULL;
> +    virDomainDef *persistentDef = NULL;
>      g_autoptr(virDomainDef) vmdef = NULL;
> -    g_autoptr(virDomainDeviceDef) dev = NULL;
> +    g_autoptr(virDomainDeviceDef) dev_live = NULL;
> +    g_autoptr(virDomainDeviceDef) dev_config = NULL;
>      int ret = -1;
>      unsigned int parse_flags = 0;
>  
> -    virCheckFlags(VIR_DOMAIN_AFFECT_CONFIG, -1);
> +    virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
> +                  VIR_DOMAIN_AFFECT_CONFIG, -1);
>  
>      if (!(vm = testDomObjFromDomain(dom)))
>          goto cleanup;
> @@ -10337,9 +10341,24 @@ testDomainUpdateDeviceFlags(virDomainPtr dom,
>          parse_flags |= VIR_DOMAIN_DEF_PARSE_INACTIVE;
>      }
>  
> -    if (!(dev = virDomainDeviceDefParse(xml, vm->def, driver->xmlopt,
> -                                        NULL, parse_flags))) {
> -        goto endjob;
> +    /*
> +     * NB: this has diverged from qemuDomainUpdateDeviceFlags(), which uses
> +     * vm->def in both cases; technically the qemu driver should do the same.
> +     */

I'll post a patch for this shortly. Let me remove it, it something that
belongs into commit message if anything.

> +    if (virDomainObjGetDefs(vm, flags, &def, &persistentDef) < 0)
> +        goto cleanup;
> +
> +    if (def) {
> +        if (!(dev_live = virDomainDeviceDefParse(xml, def, driver->xmlopt,
> +                                                 NULL, parse_flags)))
> +            goto endjob;
> +    }
> +
> +    if (persistentDef) {
> +        if (!(dev_config = virDomainDeviceDefParse(xml, persistentDef,
> +                                                   driver->xmlopt, NULL,
> +                                                   parse_flags)))
> +            goto endjob;
>      }
>  
>      if (flags & VIR_DOMAIN_AFFECT_CONFIG) {


Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

Michal

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.