11:59 p.m.
At 04/16/2011 11:28 PM, Eric Blake Write:
There's still work to add persistent callback functions, and to
make sure this all works, but this demonstrates how having a
single function makes it easy to support flags for all three
types of device modifications.
* src/qemu/qemu_driver.c (qemuDomainModifyDeviceFlags): Add
parameter, and support VIR_DOMAIN_DEVICE_MODIFY_CURRENT.
(qemuDomainAttachDeviceFlags, qemuDomainUpdateDeviceFlags)
(qemuDomainDetachDeviceFlags): Update callers.
---
After this point, we can use Kame's patch 1/4 to add in the
persistent function callbacks (with slight tweaks to their
signatures), and 2/4 to make it easier to to temporary
modifications to a config:
if (flags & CONFIG) {
create temporary def
call persistent callback
}
if (flags & LIVE) {
call live callback
}
if (no errors)
commit temporary def and live state changes, as needed
But with the benefit that CURRENT support is now provided.
src/qemu/qemu_driver.c | 47 +++++++++++++++++++++++++++++++++++------------
1 files changed, 35 insertions(+), 12 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 4f0a057..8c978be 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4031,7 +4031,8 @@ cleanup:
static int
qemuDomainModifyDeviceFlags(virDomainPtr dom, const char *xml,
unsigned int flags,
- qemuDomainModifyDeviceCallback cb)
+ qemuDomainModifyDeviceCallback live,
+ qemuDomainModifyDeviceCallback persistent)
{
struct qemud_driver *driver = dom->conn->privateData;
virDomainObjPtr vm;
@@ -4039,12 +4040,7 @@ qemuDomainModifyDeviceFlags(virDomainPtr dom, const char *xml,
virBitmapPtr qemuCaps = NULL;
int ret = -1;
bool force = (flags & VIR_DOMAIN_DEVICE_MODIFY_FORCE) != 0;
-
- if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) {
- qemuReportError(VIR_ERR_OPERATION_INVALID, "%s",
- _("cannot modify domain persistent configuration"));
- return -1;
- }
+ bool active;
qemuDriverLock(driver);
vm = virDomainFindByUUID(&driver->domains, dom->uuid);
@@ -4059,12 +4055,32 @@ qemuDomainModifyDeviceFlags(virDomainPtr dom, const char *xml,
if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0)
goto cleanup;
- if (!virDomainObjIsActive(vm)) {
+ active = virDomainObjIsActive(vm);
+
+ if ((flags & (VIR_DOMAIN_DEVICE_MODIFY_LIVE
+ | VIR_DOMAIN_DEVICE_MODIFY_CONFIG)) == 0)
+ flags |= (active ? VIR_DOMAIN_DEVICE_MODIFY_LIVE
+ : VIR_DOMAIN_DEVICE_MODIFY_CONFIG);
+
+ if ((flags & VIR_DOMAIN_DEVICE_MODIFY_LIVE) && !active) {
qemuReportError(VIR_ERR_OPERATION_INVALID,
"%s", _("cannot modify device on inactive
domain"));
goto endjob;
}
+ if ((flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) && !vm->persistent) {
+ qemuReportError(VIR_ERR_OPERATION_INVALID,
+ "%s", _("cannot modify device on transient
domain"));
+ goto endjob;
+ }
+
+ /* XXX add persistent support */
+ if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) {
kamezawa-san is writing patch to support persistent device modification(attach/detach)
and Hu Tao is writing patch to support persistent device modification(update).
We can detect whether persistent device modification is supported like this:
if ((flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) && !persistent) {
So the caller can pass NULL when persistent device modification is not supported safely.
+ qemuReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("cannot modify domain persistent configuration"));
+ return -1;
We should goto endjob to do some cleanup.
+ }
+
dev = virDomainDeviceDefParse(driver->caps, vm->def, xml,
VIR_DOMAIN_XML_INACTIVE);
if (dev == NULL)
@@ -4075,7 +4091,11 @@ qemuDomainModifyDeviceFlags(virDomainPtr dom, const char *xml,
&qemuCaps) < 0)
goto endjob;
- ret = (cb)(dom, driver, vm, dev, qemuCaps, force);
+ ret = 0;
+ if (flags & VIR_DOMAIN_DEVICE_MODIFY_LIVE)
+ ret = (live)(dom, driver, vm, dev, qemuCaps, force);
+ if (ret == 0 && (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG))
+ ret = (persistent)(dom, driver, vm, dev, qemuCaps, force);
/* update domain status forcibly because the domain status may be changed
* even if we attach the device failed. For example, a new controller may
@@ -4105,7 +4125,8 @@ qemuDomainAttachDeviceFlags(virDomainPtr dom, const char *xml,
virCheckFlags(VIR_DOMAIN_DEVICE_MODIFY_LIVE |
VIR_DOMAIN_DEVICE_MODIFY_CONFIG, -1);
return qemuDomainModifyDeviceFlags(dom, xml, flags,
- qemuDomainAttachDeviceLive);
+ qemuDomainAttachDeviceLive,
+ NULL);
}
static int
@@ -4124,7 +4145,8 @@ qemuDomainUpdateDeviceFlags(virDomainPtr dom, const char *xml,
VIR_DOMAIN_DEVICE_MODIFY_CONFIG |
VIR_DOMAIN_DEVICE_MODIFY_FORCE, -1);
return qemuDomainModifyDeviceFlags(dom, xml, flags,
- qemuDomainUpdateDeviceLive);
+ qemuDomainUpdateDeviceLive,
+ NULL);
}
@@ -4135,7 +4157,8 @@ qemuDomainDetachDeviceFlags(virDomainPtr dom, const char *xml,
virCheckFlags(VIR_DOMAIN_DEVICE_MODIFY_LIVE |
VIR_DOMAIN_DEVICE_MODIFY_CONFIG, -1);
return qemuDomainModifyDeviceFlags(dom, xml, flags,
- qemuDomainDetachDeviceLive);
+ qemuDomainDetachDeviceLive,
+ NULL);
}
static int
The other modification and the other 3 patches look good to me.