7:42 p.m.
Changes in v2:
* rebased
This patch requires another one, otherwise a small conflict appears:
https://www.redhat.com/archives/libvir-list/2016-July/msg00402.html
In qemu_driver.c, two functions were split to have less responsibility.
They were also modified so that they could be used in qemuhotplugtest.
Now instead of running different functions to attach different device
types, a generic function is run. Hopefully this will result in more
device types tested in the future.
Suggestions on how to name these two new functions are very welcome.
In the future, if one uses the following command:
virsh attach-device --live --config
the new device will have the same address assigned in both live
and config domain. To make sure that everything will work properly
after making these changes, I reworked qemuhotplug.c to work with
attaching devices to persistent domains.
So far, there is only one test for persistent attachment. I will
add more testcases as soon as I hear some feedback on these changes.
qemuhotplugtest.c should first be modified anyway, so that the three
xmls's filenames (basis domain xml, device xml, expected xml) are stated
explicitly instead of being calculated. This will allow for more
flexibility in testing and less xml files duplicates.
Tomasz
Tomasz Flendrich (10):
Change parameters to qemuDomainAttachDeviceLive
Remove an unnecessary variable
Split qemuDomainAttachDeviceFlags in two
Narrow down a parameter
Split qemuDomainDetachDeviceFlags in two
Make qemu attach/detach functions public
Narrow down a parameter to function
Use more generic functions in qemuhotplugtest
Make qemuhotplugtest work with persistent domains
Add a persistent attachment testcase
src/qemu/qemu_driver.c | 221 +++++++++++----------
src/qemu/qemu_driver.h | 14 ++
tests/qemuhotplugtest.c | 213 ++++++++++++--------
.../qemuhotplug-base-config+qemu-agent+config.xml | 45 +++++
.../qemuhotplug-base-config.xml | 40 ++++
5 files changed, 349 insertions(+), 184 deletions(-)
create mode 100644
tests/qemuhotplugtestdomains/qemuhotplug-base-config+qemu-agent+config.xml
create mode 100644 tests/qemuhotplugtestdomains/qemuhotplug-base-config.xml
--
2.7.4
7:42 p.m.
New subject: [libvirt] [PATCH v2 01/10] Change parameters to qemuDomainAttachDeviceLive
We want to be able to pass a NULL instead of the connection
and use this function in tests. To achieve this, the virConnectPtr
is passed instead of virDomainPtr, and the driver is a new separate
parameter.
---
src/qemu/qemu_driver.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index cda85f6..9d1f256 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7327,16 +7327,16 @@ qemuDomainUndefine(virDomainPtr dom)
static int
qemuDomainAttachDeviceLive(virDomainObjPtr vm,
virDomainDeviceDefPtr dev,
- virDomainPtr dom)
+ virConnectPtr conn,
+ virQEMUDriverPtr driver)
{
- virQEMUDriverPtr driver = dom->conn->privateData;
int ret = -1;
const char *alias = NULL;
switch ((virDomainDeviceType) dev->type) {
case VIR_DOMAIN_DEVICE_DISK:
qemuDomainObjCheckDiskTaint(driver, vm, dev->data.disk, NULL);
- ret = qemuDomainAttachDeviceDiskLive(dom->conn, driver, vm, dev);
+ ret = qemuDomainAttachDeviceDiskLive(conn, driver, vm, dev);
if (!ret) {
alias = dev->data.disk->info.alias;
dev->data.disk = NULL;
@@ -7369,7 +7369,7 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
case VIR_DOMAIN_DEVICE_HOSTDEV:
qemuDomainObjCheckHostdevTaint(driver, vm, dev->data.hostdev, NULL);
- ret = qemuDomainAttachHostDevice(dom->conn, driver, vm,
+ ret = qemuDomainAttachHostDevice(conn, driver, vm,
dev->data.hostdev);
if (!ret) {
alias = dev->data.hostdev->info->alias;
@@ -8151,7 +8151,8 @@ qemuDomainAttachDeviceFlags(virDomainPtr dom,
VIR_DOMAIN_DEVICE_ACTION_ATTACH) < 0)
goto endjob;
- if ((ret = qemuDomainAttachDeviceLive(vm, dev_copy, dom)) < 0)
+ if ((ret = qemuDomainAttachDeviceLive(vm, dev_copy, dom->conn,
+ dom->conn->privateData)) < 0)
goto endjob;
/*
* update domain status forcibly because the domain status may be
--
2.7.4
4:39 a.m.
New subject: [libvirt] [PATCH v2 01/10] Change parameters to qemuDomainAttachDeviceLive
On Sat, Jul 16, 2016 at 02:42:46AM +0200, Tomasz Flendrich wrote:
We want to be able to pass a NULL instead of the connection
and use this function in tests. To achieve this, the virConnectPtr
is passed instead of virDomainPtr, and the driver is a new separate
parameter.
---
src/qemu/qemu_driver.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index cda85f6..9d1f256 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -8151,7 +8151,8 @@ qemuDomainAttachDeviceFlags(virDomainPtr dom,
VIR_DOMAIN_DEVICE_ACTION_ATTACH) < 0)
goto endjob;
- if ((ret = qemuDomainAttachDeviceLive(vm, dev_copy, dom)) < 0)
+ if ((ret = qemuDomainAttachDeviceLive(vm, dev_copy, dom->conn,
+ dom->conn->privateData)) < 0)
You can pass 'driver' here, it exists here in this function. Not only
it makes the line shorter, it will not bite us if we change the type of
the function parameter.
7:42 p.m.
New subject: [libvirt] [PATCH v2 02/10] Remove an unnecessary variable
qemuCaps is no longer used anywhere in these functions,
so it can be deleted.
---
src/qemu/qemu_driver.c | 20 --------------------
1 file changed, 20 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 9d1f256..0362f0e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -8079,8 +8079,6 @@ qemuDomainAttachDeviceFlags(virDomainPtr dom,
int ret = -1;
unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE |
VIR_DOMAIN_DEF_PARSE_ABI_UPDATE;
- virQEMUCapsPtr qemuCaps = NULL;
- qemuDomainObjPrivatePtr priv;
virQEMUDriverConfigPtr cfg = NULL;
virCapsPtr caps = NULL;
@@ -8097,8 +8095,6 @@ qemuDomainAttachDeviceFlags(virDomainPtr dom,
if (!(vm = qemuDomObjFromDomain(dom)))
goto cleanup;
- priv = vm->privateData;
-
if (virDomainAttachDeviceFlagsEnsureACL(dom->conn, vm->def, flags) < 0)
goto cleanup;
@@ -8125,11 +8121,6 @@ qemuDomainAttachDeviceFlags(virDomainPtr dom,
goto endjob;
}
- if (priv->qemuCaps)
- qemuCaps = virObjectRef(priv->qemuCaps);
- else if (!(qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache,
vm->def->emulator)))
- goto endjob;
-
if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
/* Make a copy for updated domain. */
vmdef = virDomainObjCopyPersistentDef(vm, caps, driver->xmlopt);
@@ -8178,7 +8169,6 @@ qemuDomainAttachDeviceFlags(virDomainPtr dom,
qemuDomainObjEndJob(driver, vm);
cleanup:
- virObjectUnref(qemuCaps);
virDomainDefFree(vmdef);
if (dev != dev_copy)
virDomainDeviceDefFree(dev_copy);
@@ -8331,8 +8321,6 @@ qemuDomainDetachDeviceFlags(virDomainPtr dom,
virDomainDeviceDefPtr dev = NULL, dev_copy = NULL;
int ret = -1;
unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE;
- virQEMUCapsPtr qemuCaps = NULL;
- qemuDomainObjPrivatePtr priv;
virQEMUDriverConfigPtr cfg = NULL;
virCapsPtr caps = NULL;
@@ -8347,8 +8335,6 @@ qemuDomainDetachDeviceFlags(virDomainPtr dom,
if (!(vm = qemuDomObjFromDomain(dom)))
goto cleanup;
- priv = vm->privateData;
-
if (virDomainDetachDeviceFlagsEnsureACL(dom->conn, vm->def, flags) < 0)
goto cleanup;
@@ -8379,11 +8365,6 @@ qemuDomainDetachDeviceFlags(virDomainPtr dom,
goto endjob;
}
- if (priv->qemuCaps)
- qemuCaps = virObjectRef(priv->qemuCaps);
- else if (!(qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache,
vm->def->emulator)))
- goto endjob;
-
if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
/* Make a copy for updated domain. */
vmdef = virDomainObjCopyPersistentDef(vm, caps, driver->xmlopt);
@@ -8431,7 +8412,6 @@ qemuDomainDetachDeviceFlags(virDomainPtr dom,
qemuDomainObjEndJob(driver, vm);
cleanup:
- virObjectUnref(qemuCaps);
virDomainDefFree(vmdef);
if (dev != dev_copy)
virDomainDeviceDefFree(dev_copy);
--
2.7.4
7:42 p.m.
New subject: [libvirt] [PATCH v2 03/10] Split qemuDomainAttachDeviceFlags in two
Previously, qemuDomainAttachDeviceFlags was doing two things:
handling the job and attaching devices. Now the second part is
in a new function.
This change is required to make it possible to test more complex
device attachment situations, like attaching a device to both
config and live at once.
---
src/qemu/qemu_driver.c | 95 +++++++++++++++++++++++++++++---------------------
1 file changed, 56 insertions(+), 39 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 0362f0e..5dde81f 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -8066,49 +8066,34 @@ qemuDomainUpdateDeviceConfig(virDomainDefPtr vmdef,
return 0;
}
-
static int
-qemuDomainAttachDeviceFlags(virDomainPtr dom,
- const char *xml,
- unsigned int flags)
+qemuDomainAttachDeviceLiveAndConfig(virConnectPtr conn,
+ virDomainObjPtr vm,
+ virQEMUDriverPtr driver,
+ const char *xml,
+ unsigned int flags)
{
- virQEMUDriverPtr driver = dom->conn->privateData;
- virDomainObjPtr vm = NULL;
virDomainDefPtr vmdef = NULL;
+ virQEMUDriverConfigPtr cfg = NULL;
virDomainDeviceDefPtr dev = NULL, dev_copy = NULL;
int ret = -1;
+ virCapsPtr caps = NULL;
unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE |
VIR_DOMAIN_DEF_PARSE_ABI_UPDATE;
- virQEMUDriverConfigPtr cfg = NULL;
- virCapsPtr caps = NULL;
virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
VIR_DOMAIN_AFFECT_CONFIG, -1);
- virNWFilterReadLockFilterUpdates();
-
cfg = virQEMUDriverGetConfig(driver);
if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
goto cleanup;
- if (!(vm = qemuDomObjFromDomain(dom)))
- goto cleanup;
-
- if (virDomainAttachDeviceFlagsEnsureACL(dom->conn, vm->def, flags) < 0)
- goto cleanup;
-
- if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
- goto cleanup;
-
- if (virDomainObjUpdateModificationImpact(vm, &flags) < 0)
- goto endjob;
-
dev = dev_copy = virDomainDeviceDefParse(xml, vm->def,
caps, driver->xmlopt,
parse_flags);
if (dev == NULL)
- goto endjob;
+ goto cleanup;
if (flags & VIR_DOMAIN_AFFECT_CONFIG &&
flags & VIR_DOMAIN_AFFECT_LIVE) {
@@ -8118,33 +8103,32 @@ qemuDomainAttachDeviceFlags(virDomainPtr dom,
*/
dev_copy = virDomainDeviceDefCopy(dev, vm->def, caps, driver->xmlopt);
if (!dev_copy)
- goto endjob;
+ goto cleanup;
}
if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
/* Make a copy for updated domain. */
vmdef = virDomainObjCopyPersistentDef(vm, caps, driver->xmlopt);
if (!vmdef)
- goto endjob;
+ goto cleanup;
if (virDomainDefCompatibleDevice(vmdef, dev,
VIR_DOMAIN_DEVICE_ACTION_ATTACH) < 0)
- goto endjob;
-
- if ((ret = qemuDomainAttachDeviceConfig(vmdef, dev, dom->conn, caps,
+ goto cleanup;
+ if ((ret = qemuDomainAttachDeviceConfig(vmdef, dev, conn, caps,
parse_flags,
driver->xmlopt)) < 0)
- goto endjob;
+ goto cleanup;
}
if (flags & VIR_DOMAIN_AFFECT_LIVE) {
if (virDomainDefCompatibleDevice(vm->def, dev_copy,
VIR_DOMAIN_DEVICE_ACTION_ATTACH) < 0)
- goto endjob;
+ goto cleanup;
- if ((ret = qemuDomainAttachDeviceLive(vm, dev_copy, dom->conn,
- dom->conn->privateData)) < 0)
- goto endjob;
+ if ((ret = qemuDomainAttachDeviceLive(vm, dev_copy, conn,
+ driver)) < 0)
+ goto cleanup;
/*
* update domain status forcibly because the domain status may be
* changed even if we failed to attach the device. For example,
@@ -8152,7 +8136,7 @@ qemuDomainAttachDeviceFlags(virDomainPtr dom,
*/
if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps)
< 0) {
ret = -1;
- goto endjob;
+ goto cleanup;
}
}
@@ -8165,17 +8149,50 @@ qemuDomainAttachDeviceFlags(virDomainPtr dom,
}
}
- endjob:
- qemuDomainObjEndJob(driver, vm);
-
cleanup:
virDomainDefFree(vmdef);
if (dev != dev_copy)
virDomainDeviceDefFree(dev_copy);
virDomainDeviceDefFree(dev);
- virDomainObjEndAPI(&vm);
- virObjectUnref(caps);
virObjectUnref(cfg);
+ virObjectUnref(caps);
+
+ return ret;
+}
+
+static int
+qemuDomainAttachDeviceFlags(virDomainPtr dom,
+ const char *xml,
+ unsigned int flags)
+{
+ virQEMUDriverPtr driver = dom->conn->privateData;
+ virDomainObjPtr vm = NULL;
+ int ret = -1;
+
+ virNWFilterReadLockFilterUpdates();
+
+ if (!(vm = qemuDomObjFromDomain(dom)))
+ goto cleanup;
+
+ if (virDomainAttachDeviceFlagsEnsureACL(dom->conn, vm->def, flags) < 0)
+ goto cleanup;
+
+ if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
+ goto cleanup;
+
+ if (virDomainObjUpdateModificationImpact(vm, &flags) < 0)
+ goto endjob;
+
+ if (qemuDomainAttachDeviceLiveAndConfig(dom->conn, vm, driver, xml, flags) <
0)
+ goto endjob;
+
+ ret = 0;
+
+ endjob:
+ qemuDomainObjEndJob(driver, vm);
+
+ cleanup:
+ virDomainObjEndAPI(&vm);
virNWFilterUnlockFilterUpdates();
return ret;
}
--
2.7.4
7:42 p.m.
New subject: [libvirt] [PATCH v2 04/10] Narrow down a parameter
This will make splitting up qemuDomainDetachDeviceFlags into two
functions easier.
---
src/qemu/qemu_driver.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 5dde81f..ce22eff 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7472,9 +7472,8 @@ qemuDomainDetachDeviceControllerLive(virQEMUDriverPtr driver,
static int
qemuDomainDetachDeviceLive(virDomainObjPtr vm,
virDomainDeviceDefPtr dev,
- virDomainPtr dom)
+ virQEMUDriverPtr driver)
{
- virQEMUDriverPtr driver = dom->conn->privateData;
int ret = -1;
switch ((virDomainDeviceType) dev->type) {
@@ -8403,7 +8402,7 @@ qemuDomainDetachDeviceFlags(virDomainPtr dom,
VIR_DOMAIN_DEVICE_ACTION_DETACH) < 0)
goto endjob;
- if ((ret = qemuDomainDetachDeviceLive(vm, dev_copy, dom)) < 0)
+ if ((ret = qemuDomainDetachDeviceLive(vm, dev_copy,
dom->conn->privateData)) < 0)
goto endjob;
/*
* update domain status forcibly because the domain status may be
--
2.7.4
4:39 a.m.
New subject: [libvirt] [PATCH v2 04/10] Narrow down a parameter
On Sat, Jul 16, 2016 at 02:42:49AM +0200, Tomasz Flendrich wrote:
This will make splitting up qemuDomainDetachDeviceFlags into two
functions easier.
---
src/qemu/qemu_driver.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 5dde81f..ce22eff 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7472,9 +7472,8 @@ qemuDomainDetachDeviceControllerLive(virQEMUDriverPtr driver,
static int
qemuDomainDetachDeviceLive(virDomainObjPtr vm,
virDomainDeviceDefPtr dev,
- virDomainPtr dom)
+ virQEMUDriverPtr driver)
{
- virQEMUDriverPtr driver = dom->conn->privateData;
int ret = -1;
switch ((virDomainDeviceType) dev->type) {
@@ -8403,7 +8402,7 @@ qemuDomainDetachDeviceFlags(virDomainPtr dom,
VIR_DOMAIN_DEVICE_ACTION_DETACH) < 0)
goto endjob;
- if ((ret = qemuDomainDetachDeviceLive(vm, dev_copy, dom)) < 0)
+ if ((ret = qemuDomainDetachDeviceLive(vm, dev_copy,
dom->conn->privateData)) < 0)
As mentioned in patch #1, I'd rather see non-void pointer passed here.
goto endjob;
/*
* update domain status forcibly because the domain status may be
--
2.7.4
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
7:42 p.m.
New subject: [libvirt] [PATCH v2 05/10] Split qemuDomainDetachDeviceFlags in two
Previously, qemuDomainDetachDeviceFlags was doing two things:
handling the job and detaching devices. Now the second part is
in a new function.
---
src/qemu/qemu_driver.c | 92 +++++++++++++++++++++++++++++---------------------
1 file changed, 54 insertions(+), 38 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ce22eff..4a62e18 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -8325,40 +8325,26 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom,
return ret;
}
-
static int
-qemuDomainDetachDeviceFlags(virDomainPtr dom,
- const char *xml,
- unsigned int flags)
+qemuDomainDetachDeviceLiveAndConfig(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ const char *xml,
+ unsigned int flags)
{
- virQEMUDriverPtr driver = dom->conn->privateData;
- virDomainObjPtr vm = NULL;
- virDomainDefPtr vmdef = NULL;
+ virCapsPtr caps = NULL;
+ virQEMUDriverConfigPtr cfg = NULL;
virDomainDeviceDefPtr dev = NULL, dev_copy = NULL;
- int ret = -1;
unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE;
- virQEMUDriverConfigPtr cfg = NULL;
- virCapsPtr caps = NULL;
+ virDomainDefPtr vmdef = NULL;
+ int ret = -1;
virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
VIR_DOMAIN_AFFECT_CONFIG, -1);
- cfg = virQEMUDriverGetConfig(driver);
-
if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
goto cleanup;
- if (!(vm = qemuDomObjFromDomain(dom)))
- goto cleanup;
-
- if (virDomainDetachDeviceFlagsEnsureACL(dom->conn, vm->def, flags) < 0)
- goto cleanup;
-
- if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
- goto cleanup;
-
- if (virDomainObjUpdateModificationImpact(vm, &flags) < 0)
- goto endjob;
+ cfg = virQEMUDriverGetConfig(driver);
if ((flags & VIR_DOMAIN_AFFECT_CONFIG) &&
!(flags & VIR_DOMAIN_AFFECT_LIVE))
@@ -8368,7 +8354,7 @@ qemuDomainDetachDeviceFlags(virDomainPtr dom,
caps, driver->xmlopt,
parse_flags);
if (dev == NULL)
- goto endjob;
+ goto cleanup;
if (flags & VIR_DOMAIN_AFFECT_CONFIG &&
flags & VIR_DOMAIN_AFFECT_LIVE) {
@@ -8378,32 +8364,32 @@ qemuDomainDetachDeviceFlags(virDomainPtr dom,
*/
dev_copy = virDomainDeviceDefCopy(dev, vm->def, caps, driver->xmlopt);
if (!dev_copy)
- goto endjob;
+ goto cleanup;
}
if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
/* Make a copy for updated domain. */
vmdef = virDomainObjCopyPersistentDef(vm, caps, driver->xmlopt);
if (!vmdef)
- goto endjob;
+ goto cleanup;
if (virDomainDefCompatibleDevice(vmdef, dev,
VIR_DOMAIN_DEVICE_ACTION_DETACH) < 0)
- goto endjob;
+ goto cleanup;
if ((ret = qemuDomainDetachDeviceConfig(vmdef, dev, caps,
parse_flags,
driver->xmlopt)) < 0)
- goto endjob;
+ goto cleanup;
}
if (flags & VIR_DOMAIN_AFFECT_LIVE) {
if (virDomainDefCompatibleDevice(vm->def, dev_copy,
VIR_DOMAIN_DEVICE_ACTION_DETACH) < 0)
- goto endjob;
+ goto cleanup;
- if ((ret = qemuDomainDetachDeviceLive(vm, dev_copy,
dom->conn->privateData)) < 0)
- goto endjob;
+ if ((ret = qemuDomainDetachDeviceLive(vm, dev_copy, driver)) < 0)
+ goto cleanup;
/*
* update domain status forcibly because the domain status may be
* changed even if we failed to attach the device. For example,
@@ -8411,7 +8397,7 @@ qemuDomainDetachDeviceFlags(virDomainPtr dom,
*/
if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps)
< 0) {
ret = -1;
- goto endjob;
+ goto cleanup;
}
}
@@ -8424,17 +8410,47 @@ qemuDomainDetachDeviceFlags(virDomainPtr dom,
}
}
- endjob:
- qemuDomainObjEndJob(driver, vm);
-
cleanup:
- virDomainDefFree(vmdef);
+ virObjectUnref(caps);
+ virObjectUnref(cfg);
if (dev != dev_copy)
virDomainDeviceDefFree(dev_copy);
virDomainDeviceDefFree(dev);
+ virDomainDefFree(vmdef);
+ return ret;
+}
+
+static int
+qemuDomainDetachDeviceFlags(virDomainPtr dom,
+ const char *xml,
+ unsigned int flags)
+{
+ virQEMUDriverPtr driver = dom->conn->privateData;
+ virDomainObjPtr vm = NULL;
+ int ret = -1;
+
+ if (!(vm = qemuDomObjFromDomain(dom)))
+ goto cleanup;
+
+ if (virDomainDetachDeviceFlagsEnsureACL(dom->conn, vm->def, flags) < 0)
+ goto cleanup;
+
+ if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
+ goto cleanup;
+
+ if (virDomainObjUpdateModificationImpact(vm, &flags) < 0)
+ goto endjob;
+
+ if (qemuDomainDetachDeviceLiveAndConfig(driver, vm, xml, flags) < 0)
+ goto endjob;
+
+ ret = 0;
+
+ endjob:
+ qemuDomainObjEndJob(driver, vm);
+
+ cleanup:
virDomainObjEndAPI(&vm);
- virObjectUnref(caps);
- virObjectUnref(cfg);
return ret;
}
--
2.7.4
7:42 p.m.
New subject: [libvirt] [PATCH v2 06/10] Make qemu attach/detach functions public
They will be used to test device attachment and detachment, so
they have to be visible to the outside.
---
src/qemu/qemu_driver.c | 4 ++--
src/qemu/qemu_driver.h | 14 ++++++++++++++
2 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 4a62e18..0671d24 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -8065,7 +8065,7 @@ qemuDomainUpdateDeviceConfig(virDomainDefPtr vmdef,
return 0;
}
-static int
+int
qemuDomainAttachDeviceLiveAndConfig(virConnectPtr conn,
virDomainObjPtr vm,
virQEMUDriverPtr driver,
@@ -8325,7 +8325,7 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom,
return ret;
}
-static int
+int
qemuDomainDetachDeviceLiveAndConfig(virQEMUDriverPtr driver,
virDomainObjPtr vm,
const char *xml,
diff --git a/src/qemu/qemu_driver.h b/src/qemu/qemu_driver.h
index df7533a..3284933 100644
--- a/src/qemu/qemu_driver.h
+++ b/src/qemu/qemu_driver.h
@@ -24,6 +24,20 @@
#ifndef __QEMU_DRIVER_H__
# define __QEMU_DRIVER_H__
+# include "domain_conf.h"
+# include "qemu_conf.h"
+
int qemuRegister(void);
+int qemuDomainAttachDeviceLiveAndConfig(virConnectPtr conn,
+ virDomainObjPtr vm,
+ virQEMUDriverPtr driver,
+ const char *xml,
+ unsigned int flags);
+
+int qemuDomainDetachDeviceLiveAndConfig(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ const char *xml,
+ unsigned int flags);
+
#endif /* __QEMU_DRIVER_H__ */
--
2.7.4
4:39 a.m.
New subject: [libvirt] [PATCH v2 06/10] Make qemu attach/detach functions public
On Sat, Jul 16, 2016 at 02:42:51AM +0200, Tomasz Flendrich wrote:
They will be used to test device attachment and detachment, so
they have to be visible to the outside.
---
src/qemu/qemu_driver.c | 4 ++--
src/qemu/qemu_driver.h | 14 ++++++++++++++
2 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 4a62e18..0671d24 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -8065,7 +8065,7 @@ qemuDomainUpdateDeviceConfig(virDomainDefPtr vmdef,
return 0;
}
-static int
+int
qemuDomainAttachDeviceLiveAndConfig(virConnectPtr conn,
virDomainObjPtr vm,
virQEMUDriverPtr driver,
@@ -8325,7 +8325,7 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom,
return ret;
}
-static int
+int
qemuDomainDetachDeviceLiveAndConfig(virQEMUDriverPtr driver,
virDomainObjPtr vm,
const char *xml,
diff --git a/src/qemu/qemu_driver.h b/src/qemu/qemu_driver.h
index df7533a..3284933 100644
--- a/src/qemu/qemu_driver.h
+++ b/src/qemu/qemu_driver.h
@@ -24,6 +24,20 @@
#ifndef __QEMU_DRIVER_H__
# define __QEMU_DRIVER_H__
+# include "domain_conf.h"
+# include "qemu_conf.h"
+
int qemuRegister(void);
+int qemuDomainAttachDeviceLiveAndConfig(virConnectPtr conn,
+ virDomainObjPtr vm,
+ virQEMUDriverPtr driver,
+ const char *xml,
+ unsigned int flags);
+
+int qemuDomainDetachDeviceLiveAndConfig(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ const char *xml,
+ unsigned int flags);
+
#endif /* __QEMU_DRIVER_H__ */
This should not be exposed in qemu_driver.h, instead the functions
should be moved to qemu_domain.h if possible, but they now fit there.
If that's not possible, we need to do similar thing as we now have with
qemu_processpriv.h, so this could be called qemu_domainpriv.h I guess.
6:08 p.m.
New subject: [libvirt] [PATCH v2 06/10] Make qemu attach/detach functions public
+int qemuDomainAttachDeviceLiveAndConfig(virConnectPtr conn,
> + virDomainObjPtr vm,
> + virQEMUDriverPtr driver,
> + const char *xml,
> + unsigned int flags);
> +
> +int qemuDomainDetachDeviceLiveAndConfig(virQEMUDriverPtr driver,
> + virDomainObjPtr vm,
> + const char *xml,
> + unsigned int flags);
> +
> #endif /* __QEMU_DRIVER_H__ */
>
This should not be exposed in qemu_driver.h, instead the functions
should be moved to qemu_domain.h if possible, but they now fit there.
If that's not possible, we need to do similar thing as we now have with
qemu_processpriv.h, so this could be called qemu_domainpriv.h I guess
Did you mean qemu_driverpriv.h?
As you predicted, it's not easily done.
qemuDomainAttachDeviceLiveAndConfig() indirectly uses a few functions
from qemu_hotplug. It would be pointless to move them all to
qemu_domain.h, because then we would have to drag the whole
qemu_hotplug.[hc]
along.
What can be done is moving these to qemu_hotplug.h:
- qemuDomainAttachDeviceLive()
- qemuDomainDetachDeviceLive(),
and these to qemu_domain.h:
- qemuDomainAttachDeviceConfig()
- qemuDomainDetachDeviceConfig()
- qemuDomainChrPreInsert()
- qemuDomainChrInsertPreAlloced()
- qemuDomainChrInsertPreAllocCleanup()
- qemuDomainChrInsert()
- qemuDomainChrRemove().
But then where should qemuDomainAttachDeviceLiveAndConfig() and
qemuDomainDetachDeviceLiveAndConfig() be? In that
qemu_domainpriv/qemu_driverpriv.h? And should their definitions
still be in qemu_driver.c, or in a brand new .c file?
Btw qemuDomainUpdateDeviceFlags() is still unsplit and it deserves
the same treatment as the other two *DeviceFlags functions. It will
be done in v3.
Thank you again for such thorough reviews! I really appreciate it.
Tomasz
12:27 a.m.
New subject: [libvirt] [PATCH v2 06/10] Make qemu attach/detach functions public
On Wed, Jul 27, 2016 at 01:08:17AM +0200, Tomasz Flendrich wrote:
>
> +int qemuDomainAttachDeviceLiveAndConfig(virConnectPtr conn,
>> + virDomainObjPtr vm,
>> + virQEMUDriverPtr driver,
>> + const char *xml,
>> + unsigned int flags);
>> +
>> +int qemuDomainDetachDeviceLiveAndConfig(virQEMUDriverPtr driver,
>> + virDomainObjPtr vm,
>> + const char *xml,
>> + unsigned int flags);
>> +
>> #endif /* __QEMU_DRIVER_H__ */
>>
>
> This should not be exposed in qemu_driver.h, instead the functions
> should be moved to qemu_domain.h if possible, but they now fit there.
>
> If that's not possible, we need to do similar thing as we now have with
> qemu_processpriv.h, so this could be called qemu_domainpriv.h I guess
Did you mean qemu_driverpriv.h?
I really meant 'domain' as I was thinking these are qemu_domain related,
but since they live in qemu_driver.c let's do qemu_driverpriv.h in order
not to complicate things.
As you predicted, it's not easily done.
qemuDomainAttachDeviceLiveAndConfig() indirectly uses a few functions
from qemu_hotplug. It would be pointless to move them all to
qemu_domain.h, because then we would have to drag the whole
qemu_hotplug.[hc]
along.
What can be done is moving these to qemu_hotplug.h:
- qemuDomainAttachDeviceLive()
- qemuDomainDetachDeviceLive(),
and these to qemu_domain.h:
- qemuDomainAttachDeviceConfig()
- qemuDomainDetachDeviceConfig()
- qemuDomainChrPreInsert()
- qemuDomainChrInsertPreAlloced()
- qemuDomainChrInsertPreAllocCleanup()
- qemuDomainChrInsert()
- qemuDomainChrRemove().
But then where should qemuDomainAttachDeviceLiveAndConfig() and
qemuDomainDetachDeviceLiveAndConfig() be? In that
qemu_domainpriv/qemu_driverpriv.h? And should their definitions
still be in qemu_driver.c, or in a brand new .c file?
Hoinestly? I think the code should be split in all the drivers a bit
differently. There should be xxx_driver.h as is, then xxx_driver.c with
minimum functions (ideally just the xxxRegister one) and then all static
methods from the xxx_driver.c should be moved to a different file,
e.g. xxx_apis.c and those that are needed in the driver or tests would
be made non-static without problems as it as its own header file. But
that's not what was done or will happen, so let's not complicate it and
leave it as is with just the private header for those functions needed
in tests.
Btw qemuDomainUpdateDeviceFlags() is still unsplit and it deserves
the same treatment as the other two *DeviceFlags functions. It will
be done in v3.
Cool, I can't wait!
Thank you again for such thorough reviews! I really appreciate it.
Tomasz
7:42 p.m.
New subject: [libvirt] [PATCH v2 07/10] Narrow down a parameter to function
Attaching to both live and config will soon be tested, so
testQemuHotplugCheckResult will be used on both def and newDef.
---
tests/qemuhotplugtest.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index 0a5f068..d862a0e 100644
--- a/tests/qemuhotplugtest.c
+++ b/tests/qemuhotplugtest.c
@@ -175,7 +175,7 @@ testQemuHotplugUpdate(virDomainObjPtr vm,
}
static int
-testQemuHotplugCheckResult(virDomainObjPtr vm,
+testQemuHotplugCheckResult(virDomainDefPtr def,
const char *expected,
const char *expectedFile,
bool fail)
@@ -183,11 +183,11 @@ testQemuHotplugCheckResult(virDomainObjPtr vm,
char *actual;
int ret;
- actual = virDomainDefFormat(vm->def, driver.caps,
+ actual = virDomainDefFormat(def, driver.caps,
VIR_DOMAIN_DEF_FORMAT_SECURE);
if (!actual)
return -1;
- vm->def->id = QEMU_HOTPLUG_TEST_DOMAIN_ID;
+ def->id = QEMU_HOTPLUG_TEST_DOMAIN_ID;
if (STREQ(expected, actual)) {
if (fail)
@@ -299,14 +299,14 @@ testQemuHotplug(const void *data)
VIR_FREE(dev);
}
if (ret == 0 || fail)
- ret = testQemuHotplugCheckResult(vm, result_xml,
+ ret = testQemuHotplugCheckResult(vm->def, result_xml,
result_filename, fail);
break;
case DETACH:
ret = testQemuHotplugDetach(vm, dev);
if (ret == 0 || fail)
- ret = testQemuHotplugCheckResult(vm, domain_xml,
+ ret = testQemuHotplugCheckResult(vm->def, domain_xml,
domain_filename, fail);
break;
--
2.7.4
4:39 a.m.
New subject: [libvirt] [PATCH v2 07/10] Narrow down a parameter to function
On Sat, Jul 16, 2016 at 02:42:52AM +0200, Tomasz Flendrich wrote:
Attaching to both live and config will soon be tested, so
testQemuHotplugCheckResult will be used on both def and newDef.
Or you could just pass the modification impact flag in as well and
select ->def or ->newDef based on that.
---
tests/qemuhotplugtest.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index 0a5f068..d862a0e 100644
--- a/tests/qemuhotplugtest.c
+++ b/tests/qemuhotplugtest.c
@@ -175,7 +175,7 @@ testQemuHotplugUpdate(virDomainObjPtr vm,
}
static int
-testQemuHotplugCheckResult(virDomainObjPtr vm,
+testQemuHotplugCheckResult(virDomainDefPtr def,
const char *expected,
const char *expectedFile,
bool fail)
@@ -183,11 +183,11 @@ testQemuHotplugCheckResult(virDomainObjPtr vm,
char *actual;
int ret;
- actual = virDomainDefFormat(vm->def, driver.caps,
+ actual = virDomainDefFormat(def, driver.caps,
VIR_DOMAIN_DEF_FORMAT_SECURE);
if (!actual)
return -1;
- vm->def->id = QEMU_HOTPLUG_TEST_DOMAIN_ID;
+ def->id = QEMU_HOTPLUG_TEST_DOMAIN_ID;
This should not be set for newDef, it needs to be kept at '-1' for
config updates, because 'id != -1' means the domain is running.
7:42 p.m.
New subject: [libvirt] [PATCH v2 08/10] Use more generic functions in qemuhotplugtest
More generic functions, qemuDomainAttachDeviceLiveAndConfig
and qemuDomainDetachDeviceLiveAndConfig, are now used instead
of other functions in qemuhotplugtest to attach and detach devices.
---
tests/qemuhotplugtest.c | 157 +++++++++++++++++++++++++++---------------------
1 file changed, 88 insertions(+), 69 deletions(-)
diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index d862a0e..da361a2 100644
--- a/tests/qemuhotplugtest.c
+++ b/tests/qemuhotplugtest.c
@@ -23,6 +23,7 @@
#include "qemu/qemu_conf.h"
#include "qemu/qemu_hotplug.h"
#include "qemu/qemu_hotplugpriv.h"
+#include "qemu/qemu_driver.h"
#include "qemumonitortestutils.h"
#include "testutils.h"
#include "testutilsqemu.h"
@@ -52,6 +53,7 @@ struct qemuHotplugTestData {
bool keep;
virDomainObjPtr vm;
bool deviceDeletedEvent;
+ unsigned int target;
};
static int
@@ -105,19 +107,20 @@ qemuHotplugCreateObjects(virDomainXMLOptionPtr xmlopt,
static int
testQemuHotplugAttach(virDomainObjPtr vm,
- virDomainDeviceDefPtr dev)
+ virDomainDeviceDefPtr dev,
+ const char *device_xml,
+ unsigned int target)
{
int ret = -1;
switch (dev->type) {
case VIR_DOMAIN_DEVICE_DISK:
+ case VIR_DOMAIN_DEVICE_CHR:
/* conn in only used for storage pool and secrets lookup so as long
* as we don't use any of them, passing NULL should be safe
*/
- ret = qemuDomainAttachDeviceDiskLive(NULL, &driver, vm, dev);
- break;
- case VIR_DOMAIN_DEVICE_CHR:
- ret = qemuDomainAttachChrDevice(&driver, vm, dev->data.chr);
+ ret = qemuDomainAttachDeviceLiveAndConfig(NULL, vm, &driver,
+ device_xml, target);
break;
default:
VIR_TEST_VERBOSE("device type '%s' cannot be attached\n",
@@ -130,16 +133,17 @@ testQemuHotplugAttach(virDomainObjPtr vm,
static int
testQemuHotplugDetach(virDomainObjPtr vm,
- virDomainDeviceDefPtr dev)
+ virDomainDeviceDefPtr dev,
+ const char *device_xml,
+ unsigned int target)
{
int ret = -1;
switch (dev->type) {
case VIR_DOMAIN_DEVICE_DISK:
- ret = qemuDomainDetachDeviceDiskLive(&driver, vm, dev);
- break;
case VIR_DOMAIN_DEVICE_CHR:
- ret = qemuDomainDetachChrDevice(&driver, vm, dev->data.chr);
+ ret = qemuDomainDetachDeviceLiveAndConfig(&driver, vm,
+ device_xml, target);
break;
default:
VIR_TEST_VERBOSE("device type '%s' cannot be detached\n",
@@ -225,6 +229,7 @@ testQemuHotplug(const void *data)
virCapsPtr caps = NULL;
qemuMonitorTestPtr test_mon = NULL;
qemuDomainObjPrivatePtr priv = NULL;
+ unsigned int target = test->target;
if (virAsprintf(&domain_filename,
"%s/qemuhotplugtestdomains/qemuhotplug-%s.xml",
abs_srcdir, test->domain_filename) < 0 ||
@@ -292,7 +297,7 @@ testQemuHotplug(const void *data)
switch (test->action) {
case ATTACH:
- ret = testQemuHotplugAttach(vm, dev);
+ ret = testQemuHotplugAttach(vm, dev, device_xml, target);
if (ret == 0) {
/* vm->def stolen dev->data.* so we just need to free the dev
* envelope */
@@ -304,7 +309,7 @@ testQemuHotplug(const void *data)
break;
case DETACH:
- ret = testQemuHotplugDetach(vm, dev);
+ ret = testQemuHotplugDetach(vm, dev, device_xml, target);
if (ret == 0 || fail)
ret = testQemuHotplugCheckResult(vm->def, domain_xml,
domain_filename, fail);
@@ -371,7 +376,7 @@ mymain(void)
/* wait only 100ms for DEVICE_DELETED event */
qemuDomainRemoveDeviceWaitTime = 100;
-#define DO_TEST(file, ACTION, dev, event, fial, kep, ...) \
+#define DO_TEST(file, ACTION, dev, event, fial, kep, targt, ...) \
do { \
const char *my_mon[] = { __VA_ARGS__, NULL}; \
const char *name = file " " #ACTION " " dev;
\
@@ -382,28 +387,30 @@ mymain(void)
data.mon = my_mon; \
data.keep = kep; \
data.deviceDeletedEvent = event; \
+ data.target = targt; \
if (virTestRun(name, testQemuHotplug, &data) < 0) \
ret = -1; \
} while (0)
-#define DO_TEST_ATTACH(file, dev, fial, kep, ...) \
- DO_TEST(file, ATTACH, dev, false, fial, kep, __VA_ARGS__)
+#define DO_TEST_ATTACH(file, dev, fial, kep, targt, ...) \
+ DO_TEST(file, ATTACH, dev, false, fial, kep, targt, __VA_ARGS__)
-#define DO_TEST_DETACH(file, dev, fial, kep, ...) \
- DO_TEST(file, DETACH, dev, false, fial, kep, __VA_ARGS__)
+#define DO_TEST_DETACH(file, dev, fial, kep, targt, ...) \
+ DO_TEST(file, DETACH, dev, false, fial, kep, targt, __VA_ARGS__)
-#define DO_TEST_ATTACH_EVENT(file, dev, fial, kep, ...) \
- DO_TEST(file, ATTACH, dev, true, fial, kep, __VA_ARGS__)
+#define DO_TEST_ATTACH_EVENT(file, dev, fial, kep, targt, ...) \
+ DO_TEST(file, ATTACH, dev, true, fial, kep, targt, __VA_ARGS__)
-#define DO_TEST_DETACH_EVENT(file, dev, fial, kep, ...) \
- DO_TEST(file, DETACH, dev, true, fial, kep, __VA_ARGS__)
+#define DO_TEST_DETACH_EVENT(file, dev, fial, kep, targt, ...) \
+ DO_TEST(file, DETACH, dev, true, fial, kep, targt, __VA_ARGS__)
-#define DO_TEST_UPDATE(file, dev, fial, kep, ...) \
- DO_TEST(file, UPDATE, dev, false, fial, kep, __VA_ARGS__)
+#define DO_TEST_UPDATE(file, dev, fial, kep, targt, ...) \
+ DO_TEST(file, UPDATE, dev, false, fial, kep, targt, __VA_ARGS__)
#define QMP_OK "{\"return\": {}}"
#define HMP(msg) "{\"return\": \"" msg "\"}"
+#define QOM_OK "{ \"return\": []}"
#define QMP_DEVICE_DELETED(dev) \
"{" \
@@ -418,77 +425,86 @@ mymain(void)
" }" \
"}\r\n"
- DO_TEST_UPDATE("graphics-spice", "graphics-spice-nochange",
false, false, NULL);
- DO_TEST_UPDATE("graphics-spice-timeout",
"graphics-spice-timeout-nochange", false, false,
+ DO_TEST_UPDATE("graphics-spice", "graphics-spice-nochange",
false, false, VIR_DOMAIN_AFFECT_LIVE, NULL);
+ DO_TEST_UPDATE("graphics-spice-timeout",
"graphics-spice-timeout-nochange", false, false, VIR_DOMAIN_AFFECT_LIVE,
"set_password", QMP_OK, "expire_password",
QMP_OK);
- DO_TEST_UPDATE("graphics-spice-timeout",
"graphics-spice-timeout-password", false, false,
+ DO_TEST_UPDATE("graphics-spice-timeout",
"graphics-spice-timeout-password", false, false, VIR_DOMAIN_AFFECT_LIVE,
"set_password", QMP_OK, "expire_password",
QMP_OK);
- DO_TEST_UPDATE("graphics-spice", "graphics-spice-listen", true,
false, NULL);
- DO_TEST_UPDATE("graphics-spice-listen-network",
"graphics-spice-listen-network-password", false, false,
+ DO_TEST_UPDATE("graphics-spice", "graphics-spice-listen", true,
false, VIR_DOMAIN_AFFECT_LIVE, NULL);
+ DO_TEST_UPDATE("graphics-spice-listen-network",
"graphics-spice-listen-network-password", false, false, VIR_DOMAIN_AFFECT_LIVE,
"set_password", QMP_OK, "expire_password",
QMP_OK);
/* Strange huh? Currently, only graphics can be updated :-P */
- DO_TEST_UPDATE("disk-cdrom", "disk-cdrom-nochange", true, false,
NULL);
+ DO_TEST_UPDATE("disk-cdrom", "disk-cdrom-nochange", true, false,
VIR_DOMAIN_AFFECT_LIVE, NULL);
- DO_TEST_ATTACH("console-compat-2-live", "console-virtio", false,
true,
+ DO_TEST_ATTACH("console-compat-2-live", "console-virtio", false,
true, VIR_DOMAIN_AFFECT_LIVE,
"chardev-add", "{\"return\":
{\"pty\": \"/dev/pts/26\"}}",
"device_add", QMP_OK);
- DO_TEST_DETACH("console-compat-2-live", "console-virtio", false,
false,
+ DO_TEST_DETACH("console-compat-2-live", "console-virtio", false,
false, VIR_DOMAIN_AFFECT_LIVE,
"device_del", QMP_OK,
"chardev-remove", QMP_OK);
- DO_TEST_ATTACH("base-live", "disk-virtio", false, true,
+ DO_TEST_ATTACH("base-live", "disk-virtio", false, true,
VIR_DOMAIN_AFFECT_LIVE,
"human-monitor-command", HMP("OK\\r\\n"),
"device_add", QMP_OK);
- DO_TEST_DETACH("base-live", "disk-virtio", false, false,
+ DO_TEST_DETACH("base-live", "disk-virtio", false, false,
VIR_DOMAIN_AFFECT_LIVE,
"device_del", QMP_OK,
"human-monitor-command", HMP(""));
- DO_TEST_ATTACH_EVENT("base-live", "disk-virtio", false, true,
+ DO_TEST_ATTACH_EVENT("base-live", "disk-virtio", false, true,
VIR_DOMAIN_AFFECT_LIVE,
"human-monitor-command", HMP("OK\\r\\n"),
- "device_add", QMP_OK);
- DO_TEST_DETACH("base-live", "disk-virtio", true, true,
+ "device_add", QMP_OK,
+ "qom-list", QOM_OK);
+ DO_TEST_DETACH("base-live", "disk-virtio", true, true,
VIR_DOMAIN_AFFECT_LIVE,
"device_del", QMP_OK,
+ "qom-list", QOM_OK,
"human-monitor-command", HMP(""));
- DO_TEST_DETACH("base-live", "disk-virtio", false, false,
+ DO_TEST_DETACH("base-live", "disk-virtio", false, false,
VIR_DOMAIN_AFFECT_LIVE,
"device_del", QMP_DEVICE_DELETED("virtio-disk4")
QMP_OK,
- "human-monitor-command", HMP(""));
+ "human-monitor-command", HMP(""),
+ "qom-list", QOM_OK);
- DO_TEST_ATTACH("base-live", "disk-usb", false, true,
+ DO_TEST_ATTACH("base-live", "disk-usb", false, true,
VIR_DOMAIN_AFFECT_LIVE,
"human-monitor-command", HMP("OK\\r\\n"),
"device_add", QMP_OK);
- DO_TEST_DETACH("base-live", "disk-usb", false, false,
+ DO_TEST_DETACH("base-live", "disk-usb", false, false,
VIR_DOMAIN_AFFECT_LIVE,
"device_del", QMP_OK,
"human-monitor-command", HMP(""));
- DO_TEST_ATTACH_EVENT("base-live", "disk-usb", false, true,
+ DO_TEST_ATTACH_EVENT("base-live", "disk-usb", false, true,
VIR_DOMAIN_AFFECT_LIVE,
"human-monitor-command", HMP("OK\\r\\n"),
- "device_add", QMP_OK);
- DO_TEST_DETACH("base-live", "disk-usb", true, true,
+ "device_add", QMP_OK,
+ "qom-list", QOM_OK);
+ DO_TEST_DETACH("base-live", "disk-usb", true, true,
VIR_DOMAIN_AFFECT_LIVE,
"device_del", QMP_OK,
+ "qom-list", QOM_OK,
"human-monitor-command", HMP(""));
- DO_TEST_DETACH("base-live", "disk-usb", false, false,
+ DO_TEST_DETACH("base-live", "disk-usb", false, false,
VIR_DOMAIN_AFFECT_LIVE,
"device_del", QMP_DEVICE_DELETED("usb-disk16")
QMP_OK,
- "human-monitor-command", HMP(""));
+ "human-monitor-command", HMP(""),
+ "qom-list", QOM_OK);
- DO_TEST_ATTACH("base-live", "disk-scsi", false, true,
+ DO_TEST_ATTACH("base-live", "disk-scsi", false, true,
VIR_DOMAIN_AFFECT_LIVE,
"human-monitor-command", HMP("OK\\r\\n"),
"device_add", QMP_OK);
- DO_TEST_DETACH("base-live", "disk-scsi", false, false,
+ DO_TEST_DETACH("base-live", "disk-scsi", false, false,
VIR_DOMAIN_AFFECT_LIVE,
"device_del", QMP_OK,
"human-monitor-command", HMP(""));
- DO_TEST_ATTACH_EVENT("base-live", "disk-scsi", false, true,
+ DO_TEST_ATTACH_EVENT("base-live", "disk-scsi", false, true,
VIR_DOMAIN_AFFECT_LIVE,
"human-monitor-command", HMP("OK\\r\\n"),
- "device_add", QMP_OK);
- DO_TEST_DETACH("base-live", "disk-scsi", true, true,
+ "device_add", QMP_OK,
+ "qom-list", QOM_OK);
+ DO_TEST_DETACH("base-live", "disk-scsi", true, true,
VIR_DOMAIN_AFFECT_LIVE,
"device_del", QMP_OK,
+ "qom-list", QOM_OK,
"human-monitor-command", HMP(""));
- DO_TEST_DETACH("base-live", "disk-scsi", false, false,
+ DO_TEST_DETACH("base-live", "disk-scsi", false, false,
VIR_DOMAIN_AFFECT_LIVE,
"device_del", QMP_DEVICE_DELETED("scsi0-0-0-5")
QMP_OK,
- "human-monitor-command", HMP(""));
+ "human-monitor-command", HMP(""),
+ "qom-list", QOM_OK);
- DO_TEST_ATTACH("base-without-scsi-controller-live",
"disk-scsi-2", false, true,
+ DO_TEST_ATTACH("base-without-scsi-controller-live",
"disk-scsi-2", false, true, VIR_DOMAIN_AFFECT_LIVE,
/* Four controllers added */
"device_add", QMP_OK,
"device_add", QMP_OK,
@@ -497,11 +513,11 @@ mymain(void)
"human-monitor-command", HMP("OK\\r\\n"),
/* Disk added */
"device_add", QMP_OK);
- DO_TEST_DETACH("base-with-scsi-controller-live", "disk-scsi-2",
false, false,
+ DO_TEST_DETACH("base-with-scsi-controller-live", "disk-scsi-2",
false, false, VIR_DOMAIN_AFFECT_LIVE,
"device_del", QMP_OK,
"human-monitor-command", HMP(""));
- DO_TEST_ATTACH_EVENT("base-without-scsi-controller-live",
"disk-scsi-2", false, true,
+ DO_TEST_ATTACH_EVENT("base-without-scsi-controller-live",
"disk-scsi-2", false, true, VIR_DOMAIN_AFFECT_LIVE,
/* Four controllers added */
"device_add", QMP_OK,
"device_add", QMP_OK,
@@ -509,54 +525,57 @@ mymain(void)
"device_add", QMP_OK,
"human-monitor-command", HMP("OK\\r\\n"),
/* Disk added */
- "device_add", QMP_OK);
- DO_TEST_DETACH("base-with-scsi-controller-live", "disk-scsi-2",
true, true,
+ "device_add", QMP_OK,
+ "qom-list", QOM_OK);
+ DO_TEST_DETACH("base-with-scsi-controller-live", "disk-scsi-2",
true, true, VIR_DOMAIN_AFFECT_LIVE,
"device_del", QMP_OK,
+ "qom-list", QOM_OK,
"human-monitor-command", HMP(""));
- DO_TEST_DETACH("base-with-scsi-controller-live", "disk-scsi-2",
false, false,
+ DO_TEST_DETACH("base-with-scsi-controller-live", "disk-scsi-2",
false, false, VIR_DOMAIN_AFFECT_LIVE,
"device_del", QMP_DEVICE_DELETED("scsi3-0-5-7")
QMP_OK,
- "human-monitor-command", HMP(""));
+ "human-monitor-command", HMP(""),
+ "qom-list", QOM_OK);
- DO_TEST_ATTACH("base-live", "qemu-agent", false, true,
+ DO_TEST_ATTACH("base-live", "qemu-agent", false, true,
VIR_DOMAIN_AFFECT_LIVE,
"chardev-add", QMP_OK,
"device_add", QMP_OK);
- DO_TEST_DETACH("base-live", "qemu-agent-detach", false, false,
+ DO_TEST_DETACH("base-live", "qemu-agent-detach", false, false,
VIR_DOMAIN_AFFECT_LIVE,
"device_del", QMP_OK,
"chardev-remove", QMP_OK);
- DO_TEST_ATTACH("base-ccw-live", "ccw-virtio", false, true,
+ DO_TEST_ATTACH("base-ccw-live", "ccw-virtio", false, true,
VIR_DOMAIN_AFFECT_LIVE,
"human-monitor-command", HMP("OK\\r\\n"),
"device_add", QMP_OK);
- DO_TEST_DETACH("base-ccw-live", "ccw-virtio", false, false,
+ DO_TEST_DETACH("base-ccw-live", "ccw-virtio", false, false,
VIR_DOMAIN_AFFECT_LIVE,
"device_del", QMP_OK,
"human-monitor-command", HMP(""));
- DO_TEST_ATTACH("base-ccw-live-with-ccw-virtio", "ccw-virtio-2",
false, true,
+ DO_TEST_ATTACH("base-ccw-live-with-ccw-virtio", "ccw-virtio-2",
false, true, VIR_DOMAIN_AFFECT_LIVE,
"human-monitor-command", HMP("OK\\r\\n"),
"device_add", QMP_OK);
- DO_TEST_DETACH("base-ccw-live-with-ccw-virtio", "ccw-virtio-2",
false, false,
+ DO_TEST_DETACH("base-ccw-live-with-ccw-virtio", "ccw-virtio-2",
false, false, VIR_DOMAIN_AFFECT_LIVE,
"device_del", QMP_OK,
"human-monitor-command", HMP(""));
- DO_TEST_ATTACH("base-ccw-live-with-ccw-virtio",
"ccw-virtio-2-explicit", false, true,
+ DO_TEST_ATTACH("base-ccw-live-with-ccw-virtio",
"ccw-virtio-2-explicit", false, true, VIR_DOMAIN_AFFECT_LIVE,
"human-monitor-command", HMP("OK\\r\\n"),
"device_add", QMP_OK);
- DO_TEST_DETACH("base-ccw-live-with-ccw-virtio",
"ccw-virtio-2-explicit", false, false,
+ DO_TEST_DETACH("base-ccw-live-with-ccw-virtio",
"ccw-virtio-2-explicit", false, false, VIR_DOMAIN_AFFECT_LIVE,
"device_del", QMP_OK,
"human-monitor-command", HMP(""));
/* Attach a second device, then detach the first one. Then attach the first one
again. */
- DO_TEST_ATTACH("base-ccw-live-with-ccw-virtio",
"ccw-virtio-2-explicit", false, true,
+ DO_TEST_ATTACH("base-ccw-live-with-ccw-virtio",
"ccw-virtio-2-explicit", false, true, VIR_DOMAIN_AFFECT_LIVE,
"human-monitor-command", HMP("OK\\r\\n"),
"device_add", QMP_OK);
- DO_TEST_DETACH("base-ccw-live-with-2-ccw-virtio",
"ccw-virtio-1-explicit", false, true,
+ DO_TEST_DETACH("base-ccw-live-with-2-ccw-virtio",
"ccw-virtio-1-explicit", false, true, VIR_DOMAIN_AFFECT_LIVE,
"device_del", QMP_OK,
"human-monitor-command", HMP(""));
- DO_TEST_ATTACH("base-ccw-live-with-2-ccw-virtio",
"ccw-virtio-1-reverse", false, false,
+ DO_TEST_ATTACH("base-ccw-live-with-2-ccw-virtio",
"ccw-virtio-1-reverse", false, false, VIR_DOMAIN_AFFECT_LIVE,
"human-monitor-command", HMP("OK\\r\\n"),
"device_add", QMP_OK);
--
2.7.4
4:40 a.m.
New subject: [libvirt] [PATCH v2 08/10] Use more generic functions in qemuhotplugtest
On Sat, Jul 16, 2016 at 02:42:53AM +0200, Tomasz Flendrich wrote:
More generic functions, qemuDomainAttachDeviceLiveAndConfig
and qemuDomainDetachDeviceLiveAndConfig, are now used instead
of other functions in qemuhotplugtest to attach and detach devices.
---
tests/qemuhotplugtest.c | 157 +++++++++++++++++++++++++++---------------------
1 file changed, 88 insertions(+), 69 deletions(-)
diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index d862a0e..da361a2 100644
--- a/tests/qemuhotplugtest.c
+++ b/tests/qemuhotplugtest.c
@@ -23,6 +23,7 @@
#include "qemu/qemu_conf.h"
#include "qemu/qemu_hotplug.h"
#include "qemu/qemu_hotplugpriv.h"
+#include "qemu/qemu_driver.h"
This will be qemu_domainpriv.h based on previous patches. Or,
hopefully, just qemu_domain.h ;)
#include "qemumonitortestutils.h"
#include "testutils.h"
#include "testutilsqemu.h"
@@ -52,6 +53,7 @@ struct qemuHotplugTestData {
bool keep;
virDomainObjPtr vm;
bool deviceDeletedEvent;
+ unsigned int target;
virDomainModificationImpact would be more suitable here.
@@ -382,28 +387,30 @@ mymain(void)
data.mon = my_mon; \
data.keep = kep; \
data.deviceDeletedEvent = event; \
+ data.target = targt; \
if (virTestRun(name, testQemuHotplug, &data) < 0) \
ret = -1; \
} while (0)
-#define DO_TEST_ATTACH(file, dev, fial, kep, ...) \
- DO_TEST(file, ATTACH, dev, false, fial, kep, __VA_ARGS__)
+#define DO_TEST_ATTACH(file, dev, fial, kep, targt, ...) \
+ DO_TEST(file, ATTACH, dev, false, fial, kep, targt, __VA_ARGS__)
Worse readability of the macros, but better readability of the tests
would be if we had DO_TEST_ATTACH, DO_TEST_ATTACH_LIVE and
DO_TEST_ATTACH_CONFIG (the first one would be both live and config).
@@ -418,77 +425,86 @@ mymain(void)
" }" \
"}\r\n"
- DO_TEST_UPDATE("graphics-spice", "graphics-spice-nochange",
false, false, NULL);
- DO_TEST_UPDATE("graphics-spice-timeout",
"graphics-spice-timeout-nochange", false, false,
+ DO_TEST_UPDATE("graphics-spice", "graphics-spice-nochange",
false, false, VIR_DOMAIN_AFFECT_LIVE, NULL);
+ DO_TEST_UPDATE("graphics-spice-timeout",
"graphics-spice-timeout-nochange", false, false, VIR_DOMAIN_AFFECT_LIVE,
We would also not have such long lines :)
7:42 p.m.
New subject: [libvirt] [PATCH v2 09/10] Make qemuhotplugtest work with persistent domains
It was previously working only with attachments/detachments
to the live domain. Now it can test attaching/detaching to
the persistent domain too.
---
tests/qemuhotplugtest.c | 43 +++++++++++++++++++++++++++++++++++--------
1 file changed, 35 insertions(+), 8 deletions(-)
diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index da361a2..5cf29e5 100644
--- a/tests/qemuhotplugtest.c
+++ b/tests/qemuhotplugtest.c
@@ -234,12 +234,29 @@ testQemuHotplug(const void *data)
if (virAsprintf(&domain_filename,
"%s/qemuhotplugtestdomains/qemuhotplug-%s.xml",
abs_srcdir, test->domain_filename) < 0 ||
virAsprintf(&device_filename,
"%s/qemuhotplugtestdevices/qemuhotplug-%s.xml",
- abs_srcdir, test->device_filename) < 0 ||
- virAsprintf(&result_filename,
+ abs_srcdir, test->device_filename) < 0)
+ goto cleanup;
+
+ switch (target) {
+ case VIR_DOMAIN_AFFECT_LIVE:
+ if (virAsprintf(&result_filename,
"%s/qemuhotplugtestdomains/qemuhotplug-%s+%s.xml",
abs_srcdir, test->domain_filename,
test->device_filename) < 0)
+ goto cleanup;
+ break;
+ case VIR_DOMAIN_AFFECT_CONFIG:
+ if (virAsprintf(&result_filename,
+ "%s/qemuhotplugtestdomains/qemuhotplug-%s+%s+config.xml",
+ abs_srcdir, test->domain_filename,
+ test->device_filename) < 0)
+ goto cleanup;
+ break;
+ default:
+ VIR_TEST_VERBOSE("target can either be VIR_DOMAIN_AFFECT_LIVE"
+ " or VIR_DOMAIN_AFFECT_CONFIG\n");
goto cleanup;
+ }
if (virTestLoadFile(domain_filename, &domain_xml) < 0 ||
virTestLoadFile(device_filename, &device_xml) < 0)
@@ -303,16 +320,26 @@ testQemuHotplug(const void *data)
* envelope */
VIR_FREE(dev);
}
- if (ret == 0 || fail)
- ret = testQemuHotplugCheckResult(vm->def, result_xml,
- result_filename, fail);
+ if (ret == 0 || fail) {
+ if (target == VIR_DOMAIN_AFFECT_LIVE)
+ ret = testQemuHotplugCheckResult(vm->def, result_xml,
+ result_filename, fail);
+ else if (target == VIR_DOMAIN_AFFECT_CONFIG)
+ ret = testQemuHotplugCheckResult(vm->newDef, result_xml,
+ result_filename, fail);
+ }
break;
case DETACH:
ret = testQemuHotplugDetach(vm, dev, device_xml, target);
- if (ret == 0 || fail)
- ret = testQemuHotplugCheckResult(vm->def, domain_xml,
- domain_filename, fail);
+ if (ret == 0 || fail) {
+ if (target == VIR_DOMAIN_AFFECT_LIVE)
+ ret = testQemuHotplugCheckResult(vm->def, domain_xml,
+ domain_filename, fail);
+ else if (target == VIR_DOMAIN_AFFECT_CONFIG)
+ ret = testQemuHotplugCheckResult(vm->newDef, domain_xml,
+ domain_filename, fail);
+ }
break;
case UPDATE:
--
2.7.4
4:40 a.m.
New subject: [libvirt] [PATCH v2 09/10] Make qemuhotplugtest work with persistent domains
On Sat, Jul 16, 2016 at 02:42:54AM +0200, Tomasz Flendrich wrote:
It was previously working only with attachments/detachments
to the live domain. Now it can test attaching/detaching to
the persistent domain too.
---
tests/qemuhotplugtest.c | 43 +++++++++++++++++++++++++++++++++++--------
1 file changed, 35 insertions(+), 8 deletions(-)
diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index da361a2..5cf29e5 100644
--- a/tests/qemuhotplugtest.c
+++ b/tests/qemuhotplugtest.c
@@ -234,12 +234,29 @@ testQemuHotplug(const void *data)
if (virAsprintf(&domain_filename,
"%s/qemuhotplugtestdomains/qemuhotplug-%s.xml",
abs_srcdir, test->domain_filename) < 0 ||
virAsprintf(&device_filename,
"%s/qemuhotplugtestdevices/qemuhotplug-%s.xml",
- abs_srcdir, test->device_filename) < 0 ||
- virAsprintf(&result_filename,
+ abs_srcdir, test->device_filename) < 0)
+ goto cleanup;
+
+ switch (target) {
+ case VIR_DOMAIN_AFFECT_LIVE:
+ if (virAsprintf(&result_filename,
"%s/qemuhotplugtestdomains/qemuhotplug-%s+%s.xml",
abs_srcdir, test->domain_filename,
test->device_filename) < 0)
Indentation is off here.
+ goto cleanup;
+ break;
+ case VIR_DOMAIN_AFFECT_CONFIG:
+ if (virAsprintf(&result_filename,
+ "%s/qemuhotplugtestdomains/qemuhotplug-%s+%s+config.xml",
+ abs_srcdir, test->domain_filename,
+ test->device_filename) < 0)
+ goto cleanup;
+ break;
+ default:
No need to do default in case the switch condition is an enum, although
I'm not sure how that works with flags (non-continuous values).
+ VIR_TEST_VERBOSE("target can either be
VIR_DOMAIN_AFFECT_LIVE"
+ " or VIR_DOMAIN_AFFECT_CONFIG\n");
We should also take into account the fact that it can be both, but
should not be _CURRENT in tests. That way we don't have to do two lines
of DO_TEST_ATTACH_. Although that might cause confusion with the 'keep'
parameter.
goto cleanup;
+ }
if (virTestLoadFile(domain_filename, &domain_xml) < 0 ||
virTestLoadFile(device_filename, &device_xml) < 0)
@@ -303,16 +320,26 @@ testQemuHotplug(const void *data)
* envelope */
VIR_FREE(dev);
}
- if (ret == 0 || fail)
- ret = testQemuHotplugCheckResult(vm->def, result_xml,
- result_filename, fail);
+ if (ret == 0 || fail) {
+ if (target == VIR_DOMAIN_AFFECT_LIVE)
+ ret = testQemuHotplugCheckResult(vm->def, result_xml,
+ result_filename, fail);
+ else if (target == VIR_DOMAIN_AFFECT_CONFIG)
+ ret = testQemuHotplugCheckResult(vm->newDef, result_xml,
+ result_filename, fail);
+ }
break;
case DETACH:
ret = testQemuHotplugDetach(vm, dev, device_xml, target);
- if (ret == 0 || fail)
- ret = testQemuHotplugCheckResult(vm->def, domain_xml,
- domain_filename, fail);
+ if (ret == 0 || fail) {
+ if (target == VIR_DOMAIN_AFFECT_LIVE)
+ ret = testQemuHotplugCheckResult(vm->def, domain_xml,
+ domain_filename, fail);
+ else if (target == VIR_DOMAIN_AFFECT_CONFIG)
+ ret = testQemuHotplugCheckResult(vm->newDef, domain_xml,
+ domain_filename, fail);
+ }
break;
case UPDATE:
--
2.7.4
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
3:53 p.m.
New subject: [libvirt] [PATCH v2 09/10] Make qemuhotplugtest work with persistent domains
[...]
Indentation is off here.
Thank you for noticing that. Indeed, 4 spaces are missing.
+ goto cleanup;
> + break;
> + case VIR_DOMAIN_AFFECT_CONFIG:
> + if (virAsprintf(&result_filename,
> +
> "%s/qemuhotplugtestdomains/qemuhotplug-%s+%s+config.xml",
> + abs_srcdir, test->domain_filename,
> + test->device_filename) < 0)
> + goto cleanup;
> + break;
> + default:
>
No need to do default in case the switch condition is an enum, although
I'm not sure how that works with flags (non-continuous values).
You are right. It will make more sense when I change these enums to flags.
+ VIR_TEST_VERBOSE("target can either be VIR_DOMAIN_AFFECT_LIVE"
> + " or VIR_DOMAIN_AFFECT_CONFIG\n");
>
We should also take into account the fact that it can be both, but
should not be _CURRENT in tests. That way we don't have to do two lines
of DO_TEST_ATTACH_. Although that might cause confusion with the 'keep'
parameter.
I just wanted to make sure that both *_LIVE and *_CONFIG aren't supplied
at once, because handling them both isn't implemented yet.
Tomasz
7:42 p.m.
New subject: [libvirt] [PATCH v2 10/10] Add a persistent attachment testcase
This is the first testcase for qemuhotplugtest for attaching and
detaching a device to the persistent domain.
---
tests/qemuhotplugtest.c | 7 ++++
.../qemuhotplug-base-config+qemu-agent+config.xml | 45 ++++++++++++++++++++++
.../qemuhotplug-base-config.xml | 40 +++++++++++++++++++
3 files changed, 92 insertions(+)
create mode 100644
tests/qemuhotplugtestdomains/qemuhotplug-base-config+qemu-agent+config.xml
create mode 100644 tests/qemuhotplugtestdomains/qemuhotplug-base-config.xml
diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index 5cf29e5..1f94e67 100644
--- a/tests/qemuhotplugtest.c
+++ b/tests/qemuhotplugtest.c
@@ -606,6 +606,13 @@ mymain(void)
"human-monitor-command", HMP("OK\\r\\n"),
"device_add", QMP_OK);
+ DO_TEST_ATTACH("base-config", "qemu-agent", false, true,
VIR_DOMAIN_AFFECT_CONFIG,
+ "chardev-add", QMP_OK,
+ "device_add", QMP_OK);
+ DO_TEST_DETACH("base-config", "qemu-agent", false, false,
VIR_DOMAIN_AFFECT_CONFIG,
+ "device_del", QMP_OK,
+ "chardev-remove", QMP_OK);
+
qemuTestDriverFree(&driver);
return (ret == 0) ? EXIT_SUCCESS : EXIT_FAILURE;
}
diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-base-config+qemu-agent+config.xml
b/tests/qemuhotplugtestdomains/qemuhotplug-base-config+qemu-agent+config.xml
new file mode 100644
index 0000000..e209f67
--- /dev/null
+++ b/tests/qemuhotplugtestdomains/qemuhotplug-base-config+qemu-agent+config.xml
@@ -0,0 +1,45 @@
+<domain type='kvm'>
+ <name>hotplug</name>
+ <uuid>d091ea82-29e6-2e34-3005-f02617b36e87</uuid>
+ <memory unit='KiB'>4194304</memory>
+ <currentMemory unit='KiB'>4194304</currentMemory>
+ <vcpu placement='static'>4</vcpu>
+ <os>
+ <type arch='x86_64' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <features>
+ <acpi/>
+ <apic/>
+ <pae/>
+ </features>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>restart</on_crash>
+ <devices>
+ <emulator>/usr/libexec/qemu-kvm</emulator>
+ <controller type='usb' index='0'>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x01' function='0x2'/>
+ </controller>
+ <controller type='ide' index='0'>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x01' function='0x1'/>
+ </controller>
+ <controller type='scsi' index='0' model='virtio-scsi'>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x03' function='0x0'/>
+ </controller>
+ <controller type='pci' index='0' model='pci-root'/>
+ <controller type='virtio-serial' index='0'>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x04' function='0x0'/>
+ </controller>
+ <channel type='unix'>
+ <source mode='bind'/>
+ <target type='virtio' name='org.qemu.guest_agent.0'/>
+ <address type='virtio-serial' controller='0' bus='0'
port='1'/>
+ </channel>
+ <input type='mouse' bus='ps2'/>
+ <input type='keyboard' bus='ps2'/>
+ <memballoon model='none'/>
+ </devices>
+ <seclabel type='none' model='none'/>
+</domain>
diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-base-config.xml
b/tests/qemuhotplugtestdomains/qemuhotplug-base-config.xml
new file mode 100644
index 0000000..20ad0a5
--- /dev/null
+++ b/tests/qemuhotplugtestdomains/qemuhotplug-base-config.xml
@@ -0,0 +1,40 @@
+<domain type='kvm'>
+ <name>hotplug</name>
+ <uuid>d091ea82-29e6-2e34-3005-f02617b36e87</uuid>
+ <memory unit='KiB'>4194304</memory>
+ <currentMemory unit='KiB'>4194304</currentMemory>
+ <vcpu placement='static'>4</vcpu>
+ <os>
+ <type arch='x86_64' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <features>
+ <acpi/>
+ <apic/>
+ <pae/>
+ </features>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>restart</on_crash>
+ <devices>
+ <emulator>/usr/libexec/qemu-kvm</emulator>
+ <controller type='usb' index='0'>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x01' function='0x2'/>
+ </controller>
+ <controller type='ide' index='0'>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x01' function='0x1'/>
+ </controller>
+ <controller type='scsi' index='0' model='virtio-scsi'>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x03' function='0x0'/>
+ </controller>
+ <controller type='pci' index='0' model='pci-root'/>
+ <controller type='virtio-serial' index='0'>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x04' function='0x0'/>
+ </controller>
+ <input type='mouse' bus='ps2'/>
+ <input type='keyboard' bus='ps2'/>
+ <memballoon model='none'/>
+ </devices>
+ <seclabel type='none' model='none'/>
+</domain>
--
2.7.4
4:40 a.m.
New subject: [libvirt] [PATCH v2 10/10] Add a persistent attachment testcase
On Sat, Jul 16, 2016 at 02:42:55AM +0200, Tomasz Flendrich wrote:
This is the first testcase for qemuhotplugtest for attaching and
detaching a device to the persistent domain.
---
tests/qemuhotplugtest.c | 7 ++++
.../qemuhotplug-base-config+qemu-agent+config.xml | 45 ++++++++++++++++++++++
.../qemuhotplug-base-config.xml | 40 +++++++++++++++++++
3 files changed, 92 insertions(+)
create mode 100644
tests/qemuhotplugtestdomains/qemuhotplug-base-config+qemu-agent+config.xml
create mode 100644 tests/qemuhotplugtestdomains/qemuhotplug-base-config.xml
diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index 5cf29e5..1f94e67 100644
--- a/tests/qemuhotplugtest.c
+++ b/tests/qemuhotplugtest.c
@@ -606,6 +606,13 @@ mymain(void)
"human-monitor-command", HMP("OK\\r\\n"),
"device_add", QMP_OK);
+ DO_TEST_ATTACH("base-config", "qemu-agent", false, true,
VIR_DOMAIN_AFFECT_CONFIG,
+ "chardev-add", QMP_OK,
+ "device_add", QMP_OK);
+ DO_TEST_DETACH("base-config", "qemu-agent", false, false,
VIR_DOMAIN_AFFECT_CONFIG,
+ "device_del", QMP_OK,
+ "chardev-remove", QMP_OK);
+
qemuTestDriverFree(&driver);
return (ret == 0) ? EXIT_SUCCESS : EXIT_FAILURE;
}
diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-base-config+qemu-agent+config.xml
b/tests/qemuhotplugtestdomains/qemuhotplug-base-config+qemu-agent+config.xml
new file mode 100644
index 0000000..e209f67
--- /dev/null
+++ b/tests/qemuhotplugtestdomains/qemuhotplug-base-config+qemu-agent+config.xml
@@ -0,0 +1,45 @@
+<domain type='kvm'>
+ <name>hotplug</name>
+ <uuid>d091ea82-29e6-2e34-3005-f02617b36e87</uuid>
+ <memory unit='KiB'>4194304</memory>
+ <currentMemory unit='KiB'>4194304</currentMemory>
+ <vcpu placement='static'>4</vcpu>
+ <os>
+ <type arch='x86_64' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <features>
+ <acpi/>
+ <apic/>
+ <pae/>
+ </features>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>restart</on_crash>
+ <devices>
+ <emulator>/usr/libexec/qemu-kvm</emulator>
+ <controller type='usb' index='0'>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x01' function='0x2'/>
+ </controller>
+ <controller type='ide' index='0'>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x01' function='0x1'/>
+ </controller>
+ <controller type='scsi' index='0' model='virtio-scsi'>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x03' function='0x0'/>
+ </controller>
+ <controller type='pci' index='0' model='pci-root'/>
+ <controller type='virtio-serial' index='0'>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x04' function='0x0'/>
+ </controller>
+ <channel type='unix'>
+ <source mode='bind'/>
This line needs to be removed due to Jiri's patches.
Also the test fails for me, but that might be just another upstream
patch that was applied.
diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-base-config.xml
b/tests/qemuhotplugtestdomains/qemuhotplug-base-config.xml
new file mode 100644
index 0000000..20ad0a5
--- /dev/null
+++ b/tests/qemuhotplugtestdomains/qemuhotplug-base-config.xml
I don't think we need two base files, the live one should parse cleanly
as config one. Although having this one makes it way easier to just see
the diff of base and base+something to see the difference. OK, let's
keep it here :)
6:33 p.m.
New subject: [libvirt] [PATCH v2 10/10] Add a persistent attachment testcase
diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-base-config.xml
> b/tests/qemuhotplugtestdomains/qemuhotplug-base-config.xml
> new file mode 100644
> index 0000000..20ad0a5
> --- /dev/null
> +++ b/tests/qemuhotplugtestdomains/qemuhotplug-base-config.xml
>
I don't think we need two base files, the live one should parse cleanly
as config one. Although having this one makes it way easier to just see
the diff of base and base+something to see the difference. OK, let's
keep it here :)
I agree that a live one would work in DO_TEST_ATTACH. We need the one
with "-config" because in DO_TEST_DETACH it is used as the expected xml.
Tomasz
4:43 a.m.
On Sat, Jul 16, 2016 at 02:42:45AM +0200, Tomasz Flendrich wrote:
Changes in v2:
* rebased
This patch requires another one, otherwise a small conflict appears:
https://www.redhat.com/archives/libvir-list/2016-July/msg00402.html
In qemu_driver.c, two functions were split to have less responsibility.
They were also modified so that they could be used in qemuhotplugtest.
Now instead of running different functions to attach different device
types, a generic function is run. Hopefully this will result in more
device types tested in the future.
Suggestions on how to name these two new functions are very welcome.
In the future, if one uses the following command:
virsh attach-device --live --config
the new device will have the same address assigned in both live
and config domain. To make sure that everything will work properly
after making these changes, I reworked qemuhotplug.c to work with
attaching devices to persistent domains.
So far, there is only one test for persistent attachment. I will
add more testcases as soon as I hear some feedback on these changes.
qemuhotplugtest.c should first be modified anyway, so that the three
xmls's filenames (basis domain xml, device xml, expected xml) are stated
explicitly instead of being calculated. This will allow for more
flexibility in testing and less xml files duplicates.
ACK to patches 01-05, I'll push those in a while. I fixed up the nits
pointed out in the review for some of those. Also reworded some subject
lines to be a little more descriptive. The rest looks fine, it just
needs a tiny bit more of modification so I would rather make it go
through review again so that at least two pairs of eyes see it.
Thanks a lot for your patience,
Martin
3150
days inactive
3161
days old
22 comments
2 participants
participants (2)
-
Martin Kletzander -
Tomasz Flendrich