Re: [PATCH v3 03/12] test_driver: Implement virDomainDetachDeviceFlags

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Nov 10, 2021 at 10:24:22PM +0800, Luke Yue wrote:
Introduce testDomainChgDevice for further development (just like what we
did for IOThread). And introduce testDomainDetachDeviceLiveAndConfig for
detaching devices.

Signed-off-by: Luke Yue <lukedyue@xxxxxxxxx>
---
src/test/test_driver.c | 202 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 202 insertions(+)

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index ea474d55ac..6a7eb12f77 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -10051,6 +10051,207 @@ testConnectGetAllDomainStats(virConnectPtr conn,
    return ret;
}

+static int
+testDomainDetachDeviceLiveAndConfig(virDomainDef *vmdef,
+                                    virDomainDeviceDef *dev)

My comment from the previous patch would be nicely usable here as we
could easily just make use of it.

Also I see no reason for splitting the next two patches from this one.

[...]

+
+static int
+testDomainChgDevice(virDomainPtr dom,
+                    virDomainDeviceAction action,
+                    const char *xml,
+                    const char *alias,
+                    unsigned int flags)
+{
+    testDriver *driver = dom->conn->privateData;
+    virDomainObj *vm = NULL;
+    virDomainDef *def;
+    virDomainDeviceDef *dev = NULL;
+    unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE;
+    int ret = -1;
+
+    virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
+                  VIR_DOMAIN_AFFECT_CONFIG, -1);

So here you check for both live and config, saying both of them are
supported, ...

+
+    if (!(vm = testDomObjFromDomain(dom)))
+        goto cleanup;
+
+    if (virDomainObjUpdateModificationImpact(vm, &flags) < 0)
+        goto cleanup;
+
+    if (!(def = virDomainObjGetOneDef(vm, flags)))
+        goto cleanup;
+

But here it would fail with both since you are explicitly saying you
want just one definition.

+    if (action == VIR_DOMAIN_DEVICE_ACTION_DETACH)
+        parse_flags |= VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE;
+
+    if (xml) {
+        if (!(dev = virDomainDeviceDefParse(xml, def, driver->xmlopt,
+                                            driver->caps, parse_flags)))
+            goto cleanup;
+    } else if (alias) {
+        dev = g_new0(virDomainDeviceDef, 1);
+        if (virDomainDefFindDevice(def, alias, dev, true) < 0)
+            goto cleanup;
+    }
+
+    if (dev == NULL)
+        goto cleanup;
+
+    switch (action) {
+    case VIR_DOMAIN_DEVICE_ACTION_ATTACH:
+        virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+                       _("attaching devices is not supported"));
+        goto cleanup;
+        break;
+
+    case VIR_DOMAIN_DEVICE_ACTION_DETACH:
+        if (testDomainDetachDeviceLiveAndConfig(def, dev) < 0)

And I kind of see now why you call the function "LiveAndConfig" since it
can do both based on what DomainDef you call it with.  This function
could be very easily modified to do both live and config properly.

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux