[libvirt] [PATCH 0/3] allow starting qemu in paused state
https://bugzilla.redhat.com/show_bug.cgi?id=589465 This patch series only works for creation of a transient domain from XML, because only virDomainCreateXML() had a flag parameter (the virDomainCreate() API is rather lacking, in that respect). As such, the series is minimally invasive, and can hopefully be backported easily to places like RHEL6. If desired, a later patch can add hooks (either a new XML element, or a new command comparable to autostart that can toggle the state of a hidden field associated with domains) so that virDomainCreate() can also be used to start a paused domain, but that will be more invasive. Eric Blake (3): virDomainCreateXML: support new flag qemu: allow creation of a paused domain virsh: add --paused option to create include/libvirt/libvirt.h.in | 3 ++- src/libvirt.c | 2 +- src/lxc/lxc_driver.c | 7 +++++-- src/opennebula/one_driver.c | 8 ++++++-- src/openvz/openvz_driver.c | 4 +++- src/phyp/phyp_driver.c | 4 +++- src/qemu/qemu_driver.c | 28 +++++++++++++++++++--------- src/test/test_driver.c | 4 +++- src/uml/uml_driver.c | 4 +++- src/vbox/vbox_tmpl.c | 8 ++++++-- src/xen/xend_internal.c | 4 +++- src/xenapi/xenapi_driver.c | 4 +++- tools/virsh.c | 6 +++++- tools/virsh.pod | 6 ++++-- 14 files changed, 66 insertions(+), 26 deletions(-)
* include/libvirt/libvirt.h.in (virDomainCreateFlags): Add VIR_DOMAIN_START_PAUSED. * src/libvirt.c (virDomainCreateXML): Update documentation. * src/lxc/lxc_driver.c (lxcDomainCreateAndStart): Reject new flag as unimplemented. * src/opennebula/one_driver.c (oneDomainCreateAndStart): Likewise. * src/openvz/openvz_driver.c (openvzDomainCreateXML): Likewise. * src/phyp/phyp_driver.c (phypDomainCreateAndStart): Likewise. * src/qemu/qemu_driver.c (qemudDomainCreate): Likewise. * src/test/test_driver.c (testDomainCreateXML): Likewise. * src/uml/uml_driver.c (umlDomainCreate): Likewise. * src/vbox/vbox_tmpl.c (vboxDomainCreateXML): Likewise. * src/xen/xend_internal.c (xenDaemonCreateXML): Likewise. * src/xenapi/xenapi_driver.c (xenapiDomainCreateXML): Likewise. --- This has the slight drawback that any code that was previously passing a non-zero flag will now fail, but that shouldn't be a problem in practice. include/libvirt/libvirt.h.in | 3 ++- src/libvirt.c | 2 +- src/lxc/lxc_driver.c | 7 +++++-- src/opennebula/one_driver.c | 8 ++++++-- src/openvz/openvz_driver.c | 4 +++- src/phyp/phyp_driver.c | 4 +++- src/qemu/qemu_driver.c | 4 +++- src/test/test_driver.c | 4 +++- src/uml/uml_driver.c | 4 +++- src/vbox/vbox_tmpl.c | 8 ++++++-- src/xen/xend_internal.c | 4 +++- src/xenapi/xenapi_driver.c | 4 +++- 12 files changed, 41 insertions(+), 15 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 19d5205..c806441 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -121,7 +121,8 @@ typedef virDomainInfo *virDomainInfoPtr; * Domain. */ typedef enum { - VIR_DOMAIN_NONE = 0 + VIR_DOMAIN_NONE = 0, /* Default behavior */ + VIR_DOMAIN_START_PAUSED = 0 << 1, /* Launch guest in paused state */ } virDomainCreateFlags; diff --git a/src/libvirt.c b/src/libvirt.c index 9d42c76..2754fd0 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -1917,7 +1917,7 @@ virDomainGetConnect (virDomainPtr dom) * virDomainCreateXML: * @conn: pointer to the hypervisor connection * @xmlDesc: string containing an XML description of the domain - * @flags: callers should always pass 0 + * @flags: bitwise-or of supported virDomainCreateFlags * * Launch a new guest domain, based on an XML description similar * to the one returned by virDomainGetXMLDesc() diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 14a8b2a..c3f65cb 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -1,4 +1,5 @@ /* + * Copyright (C) 2010 Red Hat, Inc. * Copyright IBM Corp. 2008 * * lxc_driver.c: linux container driver functions @@ -1404,7 +1405,7 @@ cleanup: * lxcDomainCreateAndStart: * @conn: pointer to connection * @xml: XML definition of domain - * @flags: Unused + * @flags: Must be 0 for now * * Creates a domain based on xml and starts it * @@ -1413,13 +1414,15 @@ cleanup: static virDomainPtr lxcDomainCreateAndStart(virConnectPtr conn, const char *xml, - unsigned int flags ATTRIBUTE_UNUSED) { + unsigned int flags) { lxc_driver_t *driver = conn->privateData; virDomainObjPtr vm = NULL; virDomainDefPtr def; virDomainPtr dom = NULL; virDomainEventPtr event = NULL; + virCheckFlags(0, NULL); + lxcDriverLock(driver); if (!(def = virDomainDefParseString(driver->caps, xml, VIR_DOMAIN_XML_INACTIVE))) diff --git a/src/opennebula/one_driver.c b/src/opennebula/one_driver.c index acd52c2..fd99f0b 100644 --- a/src/opennebula/one_driver.c +++ b/src/opennebula/one_driver.c @@ -1,5 +1,7 @@ /*---------------------------------------------------------------------------*/ -/* Copyright 2002-2009, Distributed Systems Architecture Group, Universidad +/* + * Copyright (C) 2010 Red Hat, Inc. + * Copyright 2002-2009, Distributed Systems Architecture Group, Universidad * Complutense de Madrid (dsa-research.org) * * This library is free software; you can redistribute it and/or @@ -435,13 +437,15 @@ return_point: static virDomainPtr oneDomainCreateAndStart(virConnectPtr conn, const char *xml, - unsigned int flags ATTRIBUTE_UNUSED) { + unsigned int flags) { one_driver_t *driver = conn->privateData; virDomainObjPtr vm = NULL; virDomainDefPtr def; virDomainPtr dom = NULL; int oneid; + virCheckFlags(0, NULL); + oneDriverLock(driver); if (!(def = virDomainDefParseString(driver->caps, xml, VIR_DOMAIN_XML_INACTIVE))) diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index ce159d0..2dd27d9 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -873,7 +873,7 @@ cleanup: static virDomainPtr openvzDomainCreateXML(virConnectPtr conn, const char *xml, - unsigned int flags ATTRIBUTE_UNUSED) + unsigned int flags) { struct openvz_driver *driver = conn->privateData; virDomainDefPtr vmdef = NULL; @@ -881,6 +881,8 @@ openvzDomainCreateXML(virConnectPtr conn, const char *xml, virDomainPtr dom = NULL; const char *progstart[] = {VZCTL, "--quiet", "start", PROGRAM_SENTINAL, NULL}; + virCheckFlags(0, NULL); + openvzDriverLock(driver); if ((vmdef = virDomainDefParseString(driver->caps, xml, VIR_DOMAIN_XML_INACTIVE)) == NULL) diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index f8bea42..0f4bc20 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -1397,7 +1397,7 @@ phypDomainDestroy(virDomainPtr dom) static virDomainPtr phypDomainCreateAndStart(virConnectPtr conn, const char *xml, - unsigned int flags ATTRIBUTE_UNUSED) + unsigned int flags) { ConnectionData *connection_data = conn->networkPrivateData; @@ -1410,6 +1410,8 @@ phypDomainCreateAndStart(virConnectPtr conn, unsigned int i = 0; char *managed_system = phyp_driver->managed_system; + virCheckFlags(0, NULL); + if (!(def = virDomainDefParseString(phyp_driver->caps, xml, VIR_DOMAIN_XML_SECURE))) goto err; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 77a9562..11bd2c2 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4181,13 +4181,15 @@ static int qemudNumDomains(virConnectPtr conn) { } static virDomainPtr qemudDomainCreate(virConnectPtr conn, const char *xml, - unsigned int flags ATTRIBUTE_UNUSED) { + unsigned int flags) { struct qemud_driver *driver = conn->privateData; virDomainDefPtr def; virDomainObjPtr vm = NULL; virDomainPtr dom = NULL; virDomainEventPtr event = NULL; + virCheckFlags(0, NULL); + qemuDriverLock(driver); if (!(def = virDomainDefParseString(driver->caps, xml, VIR_DOMAIN_XML_INACTIVE))) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 395c8c9..5b8105d 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -1287,7 +1287,7 @@ cleanup: static virDomainPtr testDomainCreateXML(virConnectPtr conn, const char *xml, - unsigned int flags ATTRIBUTE_UNUSED) + unsigned int flags) { testConnPtr privconn = conn->privateData; virDomainPtr ret = NULL; @@ -1295,6 +1295,8 @@ testDomainCreateXML(virConnectPtr conn, const char *xml, virDomainObjPtr dom = NULL; virDomainEventPtr event = NULL; + virCheckFlags(0, NULL); + testDriverLock(privconn); if ((def = virDomainDefParseString(privconn->caps, xml, VIR_DOMAIN_XML_INACTIVE)) == NULL) diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index c6d8b65..3111211 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -1262,12 +1262,14 @@ static int umlNumDomains(virConnectPtr conn) { return n; } static virDomainPtr umlDomainCreate(virConnectPtr conn, const char *xml, - unsigned int flags ATTRIBUTE_UNUSED) { + unsigned int flags) { struct uml_driver *driver = conn->privateData; virDomainDefPtr def; virDomainObjPtr vm = NULL; virDomainPtr dom = NULL; + virCheckFlags(0, NULL); + umlDriverLock(driver); if (!(def = virDomainDefParseString(driver->caps, xml, VIR_DOMAIN_XML_INACTIVE))) diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index f70f3b3..1372f96 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -991,7 +991,7 @@ cleanup: } static virDomainPtr vboxDomainCreateXML(virConnectPtr conn, const char *xml, - unsigned int flags ATTRIBUTE_UNUSED) { + unsigned int flags) { /* VirtualBox currently doesn't have support for running * virtual machines without actually defining them and thus * for time being just define new machine and start it. @@ -1000,7 +1000,11 @@ static virDomainPtr vboxDomainCreateXML(virConnectPtr conn, const char *xml, * change this behaviour to the expected one. */ - virDomainPtr dom = vboxDomainDefineXML(conn, xml); + virDomainPtr dom; + + virCheckFlags(0, NULL); + + dom = vboxDomainDefineXML(conn, xml); if (dom == NULL) return NULL; diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index 51cad92..a22d32b 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -3788,7 +3788,7 @@ xenDaemonLookupByUUID(virConnectPtr conn, const unsigned char *uuid) */ static virDomainPtr xenDaemonCreateXML(virConnectPtr conn, const char *xmlDesc, - unsigned int flags ATTRIBUTE_UNUSED) + unsigned int flags) { int ret; char *sexpr; @@ -3796,6 +3796,8 @@ xenDaemonCreateXML(virConnectPtr conn, const char *xmlDesc, xenUnifiedPrivatePtr priv; virDomainDefPtr def; + virCheckFlags(0, NULL); + priv = (xenUnifiedPrivatePtr) conn->privateData; if (!(def = virDomainDefParseString(priv->caps, diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c index 7ef03cb..e3bcb63 100644 --- a/src/xenapi/xenapi_driver.c +++ b/src/xenapi/xenapi_driver.c @@ -479,7 +479,7 @@ xenapiNumOfDomains (virConnectPtr conn) static virDomainPtr xenapiDomainCreateXML (virConnectPtr conn, const char *xmlDesc, - unsigned int flags ATTRIBUTE_UNUSED) + unsigned int flags) { xen_vm_record *record = NULL; xen_vm vm = NULL; @@ -489,6 +489,8 @@ xenapiDomainCreateXML (virConnectPtr conn, if (!caps) return NULL; + virCheckFlags(0, NULL); + virDomainDefPtr defPtr = virDomainDefParseString(caps, xmlDesc, flags); createVMRecordFromXml(conn, defPtr, &record, &vm); virDomainDefFree(defPtr); -- 1.7.0.1
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 19d5205..c806441 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -121,7 +121,8 @@ typedef virDomainInfo *virDomainInfoPtr; * Domain. */ typedef enum { - VIR_DOMAIN_NONE = 0 + VIR_DOMAIN_NONE = 0, /* Default behavior */ + VIR_DOMAIN_START_PAUSED = 0 << 1, /* Launch guest in paused state */ } virDomainCreateFlags;
I believe, you wanted to write 1 << 0 as a value of VIR_DOMAIN_START_PAUSED, didn't you? The rest of this patch looks just fine to me, ACK. Jirka
On 06/08/2010 02:42 PM, Jiri Denemark wrote:
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 19d5205..c806441 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -121,7 +121,8 @@ typedef virDomainInfo *virDomainInfoPtr; * Domain. */ typedef enum { - VIR_DOMAIN_NONE = 0 + VIR_DOMAIN_NONE = 0, /* Default behavior */ + VIR_DOMAIN_START_PAUSED = 0 << 1, /* Launch guest in paused state */ } virDomainCreateFlags;
I believe, you wanted to write 1 << 0 as a value of VIR_DOMAIN_START_PAUSED, didn't you?
Blame the fact that I was traveling yesterday? Yes, a bone-headed bug on my part; thanks for catching it, and thank goodness for the review process.
The rest of this patch looks just fine to me, ACK.
Fixed, and will push as soon as I actually complete some testing on 2/3 and 3/3 (which would have prevented posting this bug in the first place).
Jirka
-- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
https://bugzilla.redhat.com/show_bug.cgi?id=589465 Some guests (eg with badly configured grub, or Windows' installation cd) require quick response from the console user. That's why we have a "launchPaused" option in vdsm. To implement it via libvirt, we need to ask libvirt not to call qemuMonitorStartCPUs() after starting qemu. Calling virDomainStop immediately after the domain is up is inherently raceful. * src/qemu/qemu_driver.c (qemudStartVMDaemon): Add new parameter; all callers adjusted. (qemudDomainCreate): Implement support for new flag. --- src/qemu/qemu_driver.c | 26 +++++++++++++++++--------- 1 files changed, 17 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 11bd2c2..9e5872e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -154,6 +154,7 @@ static int qemudStartVMDaemon(virConnectPtr conn, struct qemud_driver *driver, virDomainObjPtr vm, const char *migrateFrom, + bool start_paused, int stdin_fd, const char *stdin_path); @@ -3302,6 +3303,7 @@ static int qemudStartVMDaemon(virConnectPtr conn, struct qemud_driver *driver, virDomainObjPtr vm, const char *migrateFrom, + bool start_paused, int stdin_fd, const char *stdin_path) { const char **argv = NULL, **tmp; @@ -3553,7 +3555,9 @@ static int qemudStartVMDaemon(virConnectPtr conn, ret = 0; } - vm->state = migrateFrom ? VIR_DOMAIN_PAUSED : VIR_DOMAIN_RUNNING; + if (migrateFrom) + start_paused = true; + vm->state = start_paused ? VIR_DOMAIN_PAUSED : VIR_DOMAIN_RUNNING; for (i = 0 ; argv[i] ; i++) VIR_FREE(argv[i]); @@ -3603,7 +3607,7 @@ static int qemudStartVMDaemon(virConnectPtr conn, goto cleanup; } - if (migrateFrom == NULL) { + if (!start_paused) { DEBUG0("Starting domain CPUs"); /* Allow the CPUS to start executing */ if (qemuMonitorStartCPUs(priv->mon, conn) < 0) { @@ -4188,7 +4192,7 @@ static virDomainPtr qemudDomainCreate(virConnectPtr conn, const char *xml, virDomainPtr dom = NULL; virDomainEventPtr event = NULL; - virCheckFlags(0, NULL); + virCheckFlags(VIR_DOMAIN_START_PAUSED, NULL); qemuDriverLock(driver); if (!(def = virDomainDefParseString(driver->caps, xml, @@ -4217,7 +4221,9 @@ static virDomainPtr qemudDomainCreate(virConnectPtr conn, const char *xml, if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0) goto cleanup; /* XXXX free the 'vm' we created ? */ - if (qemudStartVMDaemon(conn, driver, vm, NULL, -1, NULL) < 0) { + if (qemudStartVMDaemon(conn, driver, vm, NULL, + (flags & VIR_DOMAIN_START_PAUSED) != 0, + -1, NULL) < 0) { if (qemuDomainObjEndJob(vm) > 0) virDomainRemoveInactive(&driver->domains, vm); @@ -6233,7 +6239,7 @@ qemudDomainSaveImageStartVM(virConnectPtr conn, } /* Set the migration source and start it up. */ - ret = qemudStartVMDaemon(conn, driver, vm, "stdio", fd, path); + ret = qemudStartVMDaemon(conn, driver, vm, "stdio", true, fd, path); if (intermediate_pid != -1) { /* Wait for intermediate process to exit */ @@ -6692,7 +6698,7 @@ static int qemudDomainObjStart(virConnectPtr conn, goto cleanup; } - ret = qemudStartVMDaemon(conn, driver, vm, NULL, -1, NULL); + ret = qemudStartVMDaemon(conn, driver, vm, NULL, false, -1, NULL); if (ret != -1) { virDomainEventPtr event = virDomainEventNewFromObj(vm, @@ -10185,7 +10191,8 @@ qemudDomainMigratePrepareTunnel(virConnectPtr dconn, /* Start the QEMU daemon, with the same command-line arguments plus * -incoming unix:/path/to/file or exec:nc -U /path/to/file */ - internalret = qemudStartVMDaemon(dconn, driver, vm, migrateFrom, -1, NULL); + internalret = qemudStartVMDaemon(dconn, driver, vm, migrateFrom, true, + -1, NULL); VIR_FREE(migrateFrom); if (internalret < 0) { /* Note that we don't set an error here because qemudStartVMDaemon @@ -10403,7 +10410,8 @@ qemudDomainMigratePrepare2 (virConnectPtr dconn, * -incoming tcp:0.0.0.0:port */ snprintf (migrateFrom, sizeof (migrateFrom), "tcp:0.0.0.0:%d", this_port); - if (qemudStartVMDaemon (dconn, driver, vm, migrateFrom, -1, NULL) < 0) { + if (qemudStartVMDaemon (dconn, driver, vm, migrateFrom, true, + -1, NULL) < 0) { /* Note that we don't set an error here because qemudStartVMDaemon * should have already done that. */ @@ -11891,7 +11899,7 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, goto endjob; rc = qemudStartVMDaemon(snapshot->domain->conn, driver, vm, NULL, - -1, NULL); + false, -1, NULL); if (qemuDomainSnapshotSetInactive(vm, driver->snapshotDir) < 0) goto endjob; if (rc < 0) -- 1.7.0.1
https://bugzilla.redhat.com/show_bug.cgi?id=589465
Some guests (eg with badly configured grub, or Windows' installation cd) require quick response from the console user. That's why we have a "launchPaused" option in vdsm.
To implement it via libvirt, we need to ask libvirt not to call qemuMonitorStartCPUs() after starting qemu. Calling virDomainStop immediately after the domain is up is inherently raceful.
* src/qemu/qemu_driver.c (qemudStartVMDaemon): Add new parameter; all callers adjusted. (qemudDomainCreate): Implement support for new flag. --- src/qemu/qemu_driver.c | 26 +++++++++++++++++--------- 1 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 11bd2c2..9e5872e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -154,6 +154,7 @@ static int qemudStartVMDaemon(virConnectPtr conn, struct qemud_driver *driver, virDomainObjPtr vm, const char *migrateFrom, + bool start_paused, int stdin_fd, const char *stdin_path); ...
Looks fine. I don't quite like the migrateFrom and start_paused having similar meaning wrt to cpus being started or not but I don't see a better way to implement it. ACK Jirka
* tools/virsh.c (opts_create): Add --paused option. (cmdCreate): Pass appropriate flag. * tools/virsh.pod: Document it. --- I'm following my own review advice of documenting changes in virsh.pod as I make them :) tools/virsh.c | 6 +++++- tools/virsh.pod | 6 ++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 1279f41..6b05949 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1141,6 +1141,7 @@ static const vshCmdOptDef opts_create[] = { #ifndef WIN32 {"console", VSH_OT_BOOL, 0, N_("attach to console after creation")}, #endif + {"paused", VSH_OT_BOOL, 0, N_("leave the guest paused after creation")}, {NULL, 0, 0, NULL} }; @@ -1155,6 +1156,9 @@ cmdCreate(vshControl *ctl, const vshCmd *cmd) #ifndef WIN32 int console = vshCommandOptBool(cmd, "console"); #endif + unsigned int paused = (vshCommandOptBool(cmd, "paused") + ? VIR_DOMAIN_START_PAUSED + : VIR_DOMAIN_NONE); if (!vshConnectionUsability(ctl, ctl->conn, TRUE)) return FALSE; @@ -1166,7 +1170,7 @@ cmdCreate(vshControl *ctl, const vshCmd *cmd) if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0) return FALSE; - dom = virDomainCreateXML(ctl->conn, buffer, 0); + dom = virDomainCreateXML(ctl->conn, buffer, paused); VIR_FREE(buffer); if (dom != NULL) { diff --git a/tools/virsh.pod b/tools/virsh.pod index 08e361d..3cadb87 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -265,11 +265,13 @@ The option I<--disable> disables autostarting. Connect the virtual serial console for the guest. -=item B<create> I<FILE> +=item B<create> I<FILE> optional I<--console> I<--paused> Create a domain from an XML <file>. An easy way to create the XML <file> is to use the B<dumpxml> command to obtain the definition of a -pre-existing guest. +pre-existing guest. The domain will be paused if the I<--option> +is used and supported by the driver; otherwise it will be running. +If I<--console> is requested, attach to the console after creation. B<Example> -- 1.7.0.1
diff --git a/tools/virsh.c b/tools/virsh.c index 1279f41..6b05949 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1141,6 +1141,7 @@ static const vshCmdOptDef opts_create[] = { #ifndef WIN32 {"console", VSH_OT_BOOL, 0, N_("attach to console after creation")}, #endif + {"paused", VSH_OT_BOOL, 0, N_("leave the guest paused after creation")}, {NULL, 0, 0, NULL} };
@@ -1155,6 +1156,9 @@ cmdCreate(vshControl *ctl, const vshCmd *cmd) #ifndef WIN32 int console = vshCommandOptBool(cmd, "console"); #endif + unsigned int paused = (vshCommandOptBool(cmd, "paused") + ? VIR_DOMAIN_START_PAUSED + : VIR_DOMAIN_NONE);
I'd probably name this variable "flags" to avoid confusion (or the need to rename it) if new flags are added to virDomainCreateXML(). ...
diff --git a/tools/virsh.pod b/tools/virsh.pod index 08e361d..3cadb87 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -265,11 +265,13 @@ The option I<--disable> disables autostarting.
Connect the virtual serial console for the guest.
-=item B<create> I<FILE> +=item B<create> I<FILE> optional I<--console> I<--paused>
Create a domain from an XML <file>. An easy way to create the XML <file> is to use the B<dumpxml> command to obtain the definition of a -pre-existing guest. +pre-existing guest. The domain will be paused if the I<--option>
Should be "--paused" instead of "--option".
+is used and supported by the driver; otherwise it will be running. +If I<--console> is requested, attach to the console after creation.
B<Example>
ACK after making those tweaks. Jirka
On 06/08/2010 02:57 PM, Jiri Denemark wrote:
+ unsigned int paused = (vshCommandOptBool(cmd, "paused") + ? VIR_DOMAIN_START_PAUSED + : VIR_DOMAIN_NONE);
I'd probably name this variable "flags" to avoid confusion (or the need to rename it) if new flags are added to virDomainCreateXML().
...
Create a domain from an XML <file>. An easy way to create the XML <file> is to use the B<dumpxml> command to obtain the definition of a -pre-existing guest. +pre-existing guest. The domain will be paused if the I<--option>
Should be "--paused" instead of "--option".
I've squashed this in: diff --git i/tools/virsh.c w/tools/virsh.c index 6b05949..c0b6112 100644 --- i/tools/virsh.c +++ w/tools/virsh.c @@ -1156,9 +1156,7 @@ cmdCreate(vshControl *ctl, const vshCmd *cmd) #ifndef WIN32 int console = vshCommandOptBool(cmd, "console"); #endif - unsigned int paused = (vshCommandOptBool(cmd, "paused") - ? VIR_DOMAIN_START_PAUSED - : VIR_DOMAIN_NONE); + unsigned int flags = VIR_DOMAIN_NONE; if (!vshConnectionUsability(ctl, ctl->conn, TRUE)) return FALSE; @@ -1170,7 +1168,10 @@ cmdCreate(vshControl *ctl, const vshCmd *cmd) if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0) return FALSE; - dom = virDomainCreateXML(ctl->conn, buffer, paused); + if (vshCommandOptBool(cmd, "paused")) + flags |= VIR_DOMAIN_START_PAUSED; + + dom = virDomainCreateXML(ctl->conn, buffer, flags); VIR_FREE(buffer); if (dom != NULL) { diff --git i/tools/virsh.pod w/tools/virsh.pod index 3cadb87..cf27e17 100644 --- i/tools/virsh.pod +++ w/tools/virsh.pod @@ -269,7 +269,7 @@ Connect the virtual serial console for the guest. Create a domain from an XML <file>. An easy way to create the XML <file> is to use the B<dumpxml> command to obtain the definition of a -pre-existing guest. The domain will be paused if the I<--option> +pre-existing guest. The domain will be paused if the I<--paused> option is used and supported by the driver; otherwise it will be running. If I<--console> is requested, attach to the console after creation.
ACK after making those tweaks.
Thanks. I've run more tests on the result this time, then pushed all three (I got an off-list ACK for 2/3, in case this email beats Jirka's promised on-list ACK). -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On Tue, Jun 08, 2010 at 01:53:49PM -0600, Eric Blake wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=589465
This patch series only works for creation of a transient domain from XML, because only virDomainCreateXML() had a flag parameter (the virDomainCreate() API is rather lacking, in that respect). As such, the series is minimally invasive, and can hopefully be backported easily to places like RHEL6. If desired, a later patch can add hooks (either a new XML element, or a new command comparable to autostart that can toggle the state of a hidden field associated with domains) so that virDomainCreate() can also be used to start a paused domain, but that will be more invasive.
IMHO we really need to make this work for both APIs. The fact that VDSM only uses virDomainCreateXML is an application specific use case & not something we should be relying on. Since the virDomainCreate() API lacks a flags parameter, we need to define a new API virDomainCreateFlags() which it adds it. That lets us get parity of functionality between transient and persistent guests. This series looks ok, but I'd like the other API implemented before we commit this. Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On 06/09/2010 06:07 AM, Daniel P. Berrange wrote:
If desired, a later patch can add hooks (either a new XML element, or a new command comparable to autostart that can toggle the state of a hidden field associated with domains) so that virDomainCreate() can also be used to start a paused domain, but that will be more invasive.
IMHO we really need to make this work for both APIs. The fact that VDSM only uses virDomainCreateXML is an application specific use case & not something we should be relying on. Since the virDomainCreate() API lacks a flags parameter, we need to define a new API virDomainCreateFlags() which it adds it. That lets us get parity of functionality between transient and persistent guests.
This series looks ok, but I'd like the other API implemented before we commit this.
Sounds good - I'm on it! -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
If desired, a later patch can add hooks (either a new XML element, or a new command comparable to autostart that can toggle the state of a hidden field associated with domains) so that virDomainCreate() can also be used to start a paused domain, but that will be more invasive.
IMHO we really need to make this work for both APIs. The fact that VDSM only uses virDomainCreateXML is an application specific use case & not something we should be relying on. Since the virDomainCreate() API lacks a flags parameter, we need to define a new API virDomainCreateFlags() which it adds it. That lets us get parity of functionality between transient and persistent guests.
This series looks ok, but I'd like the other API implemented before we commit this.
Here we go - my first attempt at an API addition patch. More comments in some of the various patches. Also, note that I'm including a 6/5 for any discussion, but which should not be considered part of the series to be pushed. Eric Blake (6): libvirt: introduce domainCreateWithFlags API remote: protocol implementation for virDomainCreateWithFlags drivers: add virDomainCreateWithFlags if virDomainCreate exists qemu: support starting persistent domain paused virsh: add start --paused support [DON'T APPLY] remote: optimize remoteDomainCreate daemon/remote.c | 27 ++++++++++++++++++ daemon/remote_dispatch_args.h | 1 + daemon/remote_dispatch_prototypes.h | 8 +++++ daemon/remote_dispatch_ret.h | 1 + daemon/remote_dispatch_table.h | 5 +++ include/libvirt/libvirt.h.in | 2 + src/driver.h | 6 +++- src/esx/esx_driver.c | 11 ++++++- src/libvirt.c | 45 +++++++++++++++++++++++++++++- src/libvirt_public.syms | 6 ++++ src/lxc/lxc_driver.c | 21 ++++++++++++- src/opennebula/one_driver.c | 10 ++++++- src/openvz/openvz_driver.c | 11 ++++++- src/phyp/phyp_driver.c | 1 + src/qemu/qemu_driver.c | 26 +++++++++++++---- src/remote/remote_driver.c | 53 +++++++++++++++++++++++++++++++++++ src/remote/remote_protocol.c | 20 +++++++++++++ src/remote/remote_protocol.h | 16 ++++++++++ src/remote/remote_protocol.x | 14 ++++++++- src/remote_protocol-structs | 7 ++++ src/test/test_driver.c | 9 +++++- src/uml/uml_driver.c | 8 ++++- src/vbox/vbox_tmpl.c | 9 +++++- src/xen/xen_driver.c | 11 ++++++- src/xenapi/xenapi_driver.c | 20 ++++++++++++- tools/virsh.c | 9 +++++- tools/virsh.pod | 7 +++- 27 files changed, 339 insertions(+), 25 deletions(-)
Persistent domain creation needs the same features as transient domains, but virDomainCreate lacks the flags argument present in virDomainCreateXML. virDomainCreateFlags is already claimed as a public enum, so we have to break convention and expose virDomainCreateWithFlags. * include/libvirt/libvirt.h.in (virDomainCreateWithFlags): Add. * src/driver.h (virDrvDomainCreateWithFlags): Internal API. * src/libvirt.c (virDomainCreateWithFlags): Glue public API to driver API. * src/libvirt_public.syms (LIBVIRT_0.8.2): Expose public API. * src/esx/esx_driver.c (esxDriver): Add stub for driver. * src/lxc/lxc_driver.c (lxcDriver): Likewise. * src/opennebula/one_driver.c (oneDriver): Likewise. * src/openvz/openvz_driver.c (openvzDriver): Likewise. * src/phyp/phyp_driver.c (phypDriver): Likewise. * src/qemu/qemu_driver.c (qemuDriver): Likewise. * src/remote/remote_driver.c (remote_driver): Likewise. * src/test/test_driver.c (testDriver): Likewise. * src/uml/uml_driver.c (umlDriver): Likewise. * src/vbox/vbox_tmpl.c (Driver): Likewise. * src/xen/xen_driver.c (xenUnifiedDriver): Likewise. * src/xenapi/xenapi_driver.c (xenapiDriver): Likewise. --- include/libvirt/libvirt.h.in | 2 + src/driver.h | 6 ++++- src/esx/esx_driver.c | 1 + src/libvirt.c | 45 +++++++++++++++++++++++++++++++++++++++++- src/libvirt_public.syms | 6 +++++ src/lxc/lxc_driver.c | 1 + src/opennebula/one_driver.c | 1 + src/openvz/openvz_driver.c | 1 + src/phyp/phyp_driver.c | 1 + src/qemu/qemu_driver.c | 1 + src/remote/remote_driver.c | 1 + src/test/test_driver.c | 1 + src/uml/uml_driver.c | 1 + src/vbox/vbox_tmpl.c | 1 + src/xen/xen_driver.c | 1 + src/xenapi/xenapi_driver.c | 1 + 16 files changed, 69 insertions(+), 2 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 81bfa1a..b45f7ec 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -799,6 +799,8 @@ int virConnectListDefinedDomains (virConnectPtr conn, char **const names, int maxnames); int virDomainCreate (virDomainPtr domain); +int virDomainCreateWithFlags (virDomainPtr domain, + unsigned int flags); int virDomainGetAutostart (virDomainPtr domain, int *autostart); diff --git a/src/driver.h b/src/driver.h index 0975b59..22e3db6 100644 --- a/src/driver.h +++ b/src/driver.h @@ -161,6 +161,9 @@ typedef int (*virDrvNumOfDefinedDomains) (virConnectPtr conn); typedef int (*virDrvDomainCreate) (virDomainPtr dom); +typedef int + (*virDrvDomainCreateWithFlags) (virDomainPtr dom, + unsigned int flags); typedef virDomainPtr (*virDrvDomainDefineXML) (virConnectPtr conn, const char *xml); @@ -468,7 +471,7 @@ typedef int * - close */ struct _virDriver { - int no; /* the number virDrvNo */ + int no; /* the number virDrvNo */ const char * name; /* the name of the driver */ virDrvOpen open; virDrvClose close; @@ -511,6 +514,7 @@ struct _virDriver { virDrvListDefinedDomains listDefinedDomains; virDrvNumOfDefinedDomains numOfDefinedDomains; virDrvDomainCreate domainCreate; + virDrvDomainCreateWithFlags domainCreateWithFlags; virDrvDomainDefineXML domainDefineXML; virDrvDomainUndefine domainUndefine; virDrvDomainAttachDevice domainAttachDevice; diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index c5cf2e8..0b2a3b6 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -3694,6 +3694,7 @@ static virDriver esxDriver = { esxListDefinedDomains, /* listDefinedDomains */ esxNumberOfDefinedDomains, /* numOfDefinedDomains */ esxDomainCreate, /* domainCreate */ + NULL, /* domainCreateWithFlags */ esxDomainDefineXML, /* domainDefineXML */ esxDomainUndefine, /* domainUndefine */ NULL, /* domainAttachDevice */ diff --git a/src/libvirt.c b/src/libvirt.c index 2754fd0..6332ac7 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -4850,7 +4850,7 @@ error: * virDomainCreate: * @domain: pointer to a defined domain * - * launch a defined domain. If the call succeed the domain moves from the + * Launch a defined domain. If the call succeeds the domain moves from the * defined to the running domains pools. * * Returns 0 in case of success, -1 in case of error @@ -4889,6 +4889,49 @@ error: } /** + * virDomainCreateWithFlags: + * @domain: pointer to a defined domain + * @flags: bitwise-or of supported virDomainCreateFlags + * + * Launch a defined domain. If the call succeeds the domain moves from the + * defined to the running domains pools. + * + * Returns 0 in case of success, -1 in case of error + */ +int +virDomainCreateWithFlags(virDomainPtr domain, unsigned int flags) { + virConnectPtr conn; + DEBUG("domain=%p, flags=%d", domain, flags); + + virResetLastError(); + + if (!VIR_IS_CONNECTED_DOMAIN(domain)) { + virLibDomainError(NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__); + virDispatchError(NULL); + return (-1); + } + conn = domain->conn; + if (conn->flags & VIR_CONNECT_RO) { + virLibDomainError(domain, VIR_ERR_OPERATION_DENIED, __FUNCTION__); + goto error; + } + + if (conn->driver->domainCreateWithFlags) { + int ret; + ret = conn->driver->domainCreateWithFlags (domain, flags); + if (ret < 0) + goto error; + return ret; + } + + virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(domain->conn); + return -1; +} + +/** * virDomainGetAutostart: * @domain: a domain object * @autostart: the value returned diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 81465d3..849c163 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -399,4 +399,10 @@ LIBVIRT_0.8.1 { virDomainGetBlockInfo; } LIBVIRT_0.8.0; + +LIBVIRT_0.8.2 { + global: + virDomainCreateWithFlags; +} LIBVIRT_0.8.1; + # .... define new API here using predicted next version number .... diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index c3f65cb..2027abf 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -2557,6 +2557,7 @@ static virDriver lxcDriver = { lxcListDefinedDomains, /* listDefinedDomains */ lxcNumDefinedDomains, /* numOfDefinedDomains */ lxcDomainStart, /* domainCreate */ + NULL, /* domainCreateWithFlags */ lxcDomainDefine, /* domainDefineXML */ lxcDomainUndefine, /* domainUndefine */ NULL, /* domainAttachDevice */ diff --git a/src/opennebula/one_driver.c b/src/opennebula/one_driver.c index fd99f0b..caa0d67 100644 --- a/src/opennebula/one_driver.c +++ b/src/opennebula/one_driver.c @@ -755,6 +755,7 @@ static virDriver oneDriver = { oneListDefinedDomains, /* listDefinedDomains */ oneNumDefinedDomains, /* numOfDefinedDomains */ oneDomainStart, /* domainCreate */ + NULL, /* domainCreateWithFlags */ oneDomainDefine, /* domainDefineXML */ oneDomainUndefine, /* domainUndefine */ NULL, /* domainAttachDevice */ diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index 2dd27d9..8ee9ad5 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -1507,6 +1507,7 @@ static virDriver openvzDriver = { openvzListDefinedDomains, /* listDefinedDomains */ openvzNumDefinedDomains, /* numOfDefinedDomains */ openvzDomainCreate, /* domainCreate */ + NULL, /* domainCreateWithFlags */ openvzDomainDefineXML, /* domainDefineXML */ openvzDomainUndefine, /* domainUndefine */ NULL, /* domainAttachDevice */ diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index 0f4bc20..c04a487 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -1609,6 +1609,7 @@ virDriver phypDriver = { phypListDefinedDomains, /* listDefinedDomains */ phypNumDefinedDomains, /* numOfDefinedDomains */ NULL, /* domainCreate */ + NULL, /* domainCreateWithFlags */ NULL, /* domainDefineXML */ NULL, /* domainUndefine */ NULL, /* domainAttachDevice */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index bc8dcfa..760a27a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12195,6 +12195,7 @@ static virDriver qemuDriver = { qemudListDefinedDomains, /* listDefinedDomains */ qemudNumDefinedDomains, /* numOfDefinedDomains */ qemudDomainStart, /* domainCreate */ + NULL, /* domainCreateWithFlags */ qemudDomainDefine, /* domainDefineXML */ qemudDomainUndefine, /* domainUndefine */ qemudDomainAttachDevice, /* domainAttachDevice */ diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 80977a3..3f37cc8 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -10215,6 +10215,7 @@ static virDriver remote_driver = { remoteListDefinedDomains, /* listDefinedDomains */ remoteNumOfDefinedDomains, /* numOfDefinedDomains */ remoteDomainCreate, /* domainCreate */ + NULL, /* domainCreateWithFlags */ remoteDomainDefineXML, /* domainDefineXML */ remoteDomainUndefine, /* domainUndefine */ remoteDomainAttachDevice, /* domainAttachDevice */ diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 5b8105d..13eb5e0 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -5261,6 +5261,7 @@ static virDriver testDriver = { testListDefinedDomains, /* listDefinedDomains */ testNumOfDefinedDomains, /* numOfDefinedDomains */ testDomainCreate, /* domainCreate */ + NULL, /* domainCreateWithFlags */ testDomainDefineXML, /* domainDefineXML */ testDomainUndefine, /* domainUndefine */ NULL, /* domainAttachDevice */ diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 3111211..4978e48 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -1892,6 +1892,7 @@ static virDriver umlDriver = { umlListDefinedDomains, /* listDefinedDomains */ umlNumDefinedDomains, /* numOfDefinedDomains */ umlDomainStart, /* domainCreate */ + NULL, /* domainCreateWithFlags */ umlDomainDefine, /* domainDefineXML */ umlDomainUndefine, /* domainUndefine */ NULL, /* domainAttachDevice */ diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index 1372f96..3d94ce9 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -8173,6 +8173,7 @@ virDriver NAME(Driver) = { vboxListDefinedDomains, /* listDefinedDomains */ vboxNumOfDefinedDomains, /* numOfDefinedDomains */ vboxDomainCreate, /* domainCreate */ + NULL, /* domainCreateWithFlags */ vboxDomainDefineXML, /* domainDefineXML */ vboxDomainUndefine, /* domainUndefine */ vboxDomainAttachDevice, /* domainAttachDevice */ diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index 91f0acd..ca6b246 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -1941,6 +1941,7 @@ static virDriver xenUnifiedDriver = { xenUnifiedListDefinedDomains, /* listDefinedDomains */ xenUnifiedNumOfDefinedDomains, /* numOfDefinedDomains */ xenUnifiedDomainCreate, /* domainCreate */ + NULL, /* domainCreateWithFlags */ xenUnifiedDomainDefineXML, /* domainDefineXML */ xenUnifiedDomainUndefine, /* domainUndefine */ xenUnifiedDomainAttachDevice, /* domainAttachDevice */ diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c index e3bcb63..518c4a7 100644 --- a/src/xenapi/xenapi_driver.c +++ b/src/xenapi/xenapi_driver.c @@ -1744,6 +1744,7 @@ static virDriver xenapiDriver = { xenapiListDefinedDomains, /* listDefinedDomains */ xenapiNumOfDefinedDomains, /* numOfDefinedDomains */ xenapiDomainCreate, /* domainCreate */ + NULL, /* domainCreateWithFlags */ xenapiDomainDefineXML, /* domainDefineXML */ xenapiDomainUndefine, /* domainUndefine */ NULL, /* domainAttachDevice */ -- 1.7.0.1
On Thu, Jun 10, 2010 at 11:16:09AM -0600, Eric Blake wrote:
Persistent domain creation needs the same features as transient domains, but virDomainCreate lacks the flags argument present in virDomainCreateXML. virDomainCreateFlags is already claimed as a public enum, so we have to break convention and expose virDomainCreateWithFlags.
* include/libvirt/libvirt.h.in (virDomainCreateWithFlags): Add. * src/driver.h (virDrvDomainCreateWithFlags): Internal API. * src/libvirt.c (virDomainCreateWithFlags): Glue public API to driver API. * src/libvirt_public.syms (LIBVIRT_0.8.2): Expose public API. * src/esx/esx_driver.c (esxDriver): Add stub for driver. * src/lxc/lxc_driver.c (lxcDriver): Likewise. * src/opennebula/one_driver.c (oneDriver): Likewise. * src/openvz/openvz_driver.c (openvzDriver): Likewise. * src/phyp/phyp_driver.c (phypDriver): Likewise. * src/qemu/qemu_driver.c (qemuDriver): Likewise. * src/remote/remote_driver.c (remote_driver): Likewise. * src/test/test_driver.c (testDriver): Likewise. * src/uml/uml_driver.c (umlDriver): Likewise. * src/vbox/vbox_tmpl.c (Driver): Likewise. * src/xen/xen_driver.c (xenUnifiedDriver): Likewise. * src/xenapi/xenapi_driver.c (xenapiDriver): Likewise. --- include/libvirt/libvirt.h.in | 2 + src/driver.h | 6 ++++- src/esx/esx_driver.c | 1 + src/libvirt.c | 45 +++++++++++++++++++++++++++++++++++++++++- src/libvirt_public.syms | 6 +++++ src/lxc/lxc_driver.c | 1 + src/opennebula/one_driver.c | 1 + src/openvz/openvz_driver.c | 1 + src/phyp/phyp_driver.c | 1 + src/qemu/qemu_driver.c | 1 + src/remote/remote_driver.c | 1 + src/test/test_driver.c | 1 + src/uml/uml_driver.c | 1 + src/vbox/vbox_tmpl.c | 1 + src/xen/xen_driver.c | 1 + src/xenapi/xenapi_driver.c | 1 + 16 files changed, 69 insertions(+), 2 deletions(-)
ACK Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
Define the wire format for the new virDomainCreateWithFlags API, and implement client and server side of marshaling code. * daemon/remote.c (remoteDispatchDomainCreateWithFlags): Add server side dispatch for virDomainCreateWithFlags. * src/remote/remote_driver.c (remoteDomainCreateWithFlags) (remote_driver): Client side serialization. * src/remote/remote_protocol.x (remote_domain_create_with_flags_args) (remote_domain_create_with_flags_ret) (REMOTE_PROC_DOMAIN_CREATE_WITH_FLAGS): Define wire format. * daemon/remote_dispatch_args.h: Regenerate. * daemon/remote_dispatch_prototypes.h: Likewise. * daemon/remote_dispatch_table.h: Likewise. * src/remote/remote_protocol.c: Likewise. * src/remote/remote_protocol.h: Likewise. * src/remote_protocol-structs: Likewise. --- daemon/remote.c | 27 +++++++++++++++++++++++++++ daemon/remote_dispatch_args.h | 1 + daemon/remote_dispatch_prototypes.h | 8 ++++++++ daemon/remote_dispatch_ret.h | 1 + daemon/remote_dispatch_table.h | 5 +++++ src/remote/remote_driver.c | 34 +++++++++++++++++++++++++++++++++- src/remote/remote_protocol.c | 20 ++++++++++++++++++++ src/remote/remote_protocol.h | 16 ++++++++++++++++ src/remote/remote_protocol.x | 14 ++++++++++++-- src/remote_protocol-structs | 7 +++++++ 10 files changed, 130 insertions(+), 3 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index c54565c..88a5494 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -1213,6 +1213,33 @@ remoteDispatchDomainCreate (struct qemud_server *server ATTRIBUTE_UNUSED, } static int +remoteDispatchDomainCreateWithFlags (struct qemud_server *server ATTRIBUTE_UNUSED, + struct qemud_client *client ATTRIBUTE_UNUSED, + virConnectPtr conn, + remote_message_header *hdr ATTRIBUTE_UNUSED, + remote_error *rerr, + remote_domain_create_with_flags_args *args, + remote_domain_create_with_flags_ret *ret) +{ + virDomainPtr dom; + + dom = get_nonnull_domain (conn, args->dom); + if (dom == NULL) { + remoteDispatchConnError(rerr, conn); + return -1; + } + + if (virDomainCreateWithFlags (dom, args->flags) == -1) { + virDomainFree(dom); + remoteDispatchConnError(rerr, conn); + return -1; + } + ret->dom.id = dom->id; + virDomainFree(dom); + return 0; +} + +static int remoteDispatchDomainCreateXml (struct qemud_server *server ATTRIBUTE_UNUSED, struct qemud_client *client ATTRIBUTE_UNUSED, virConnectPtr conn, diff --git a/daemon/remote_dispatch_args.h b/daemon/remote_dispatch_args.h index b0c52d2..ee95043 100644 --- a/daemon/remote_dispatch_args.h +++ b/daemon/remote_dispatch_args.h @@ -164,3 +164,4 @@ remote_domain_revert_to_snapshot_args val_remote_domain_revert_to_snapshot_args; remote_domain_snapshot_delete_args val_remote_domain_snapshot_delete_args; remote_domain_get_block_info_args val_remote_domain_get_block_info_args; + remote_domain_create_with_flags_args val_remote_domain_create_with_flags_args; diff --git a/daemon/remote_dispatch_prototypes.h b/daemon/remote_dispatch_prototypes.h index 79d1eec..cf1a0f9 100644 --- a/daemon/remote_dispatch_prototypes.h +++ b/daemon/remote_dispatch_prototypes.h @@ -122,6 +122,14 @@ static int remoteDispatchDomainCreate( remote_error *err, remote_domain_create_args *args, void *ret); +static int remoteDispatchDomainCreateWithFlags( + struct qemud_server *server, + struct qemud_client *client, + virConnectPtr conn, + remote_message_header *hdr, + remote_error *err, + remote_domain_create_with_flags_args *args, + remote_domain_create_with_flags_ret *ret); static int remoteDispatchDomainCreateXml( struct qemud_server *server, struct qemud_client *client, diff --git a/daemon/remote_dispatch_ret.h b/daemon/remote_dispatch_ret.h index f9d6237..75ac0b2 100644 --- a/daemon/remote_dispatch_ret.h +++ b/daemon/remote_dispatch_ret.h @@ -134,3 +134,4 @@ remote_domain_has_current_snapshot_ret val_remote_domain_has_current_snapshot_ret; remote_domain_snapshot_current_ret val_remote_domain_snapshot_current_ret; remote_domain_get_block_info_ret val_remote_domain_get_block_info_ret; + remote_domain_create_with_flags_ret val_remote_domain_create_with_flags_ret; diff --git a/daemon/remote_dispatch_table.h b/daemon/remote_dispatch_table.h index 86bd3b0..ef00edd 100644 --- a/daemon/remote_dispatch_table.h +++ b/daemon/remote_dispatch_table.h @@ -982,3 +982,8 @@ .args_filter = (xdrproc_t) xdr_void, .ret_filter = (xdrproc_t) xdr_void, }, +{ /* DomainCreateWithFlags => 196 */ + .fn = (dispatch_fn) remoteDispatchDomainCreateWithFlags, + .args_filter = (xdrproc_t) xdr_remote_domain_create_with_flags_args, + .ret_filter = (xdrproc_t) xdr_remote_domain_create_with_flags_ret, +}, diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 3f37cc8..7052bf1 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -3036,6 +3036,38 @@ done: return rv; } +static int +remoteDomainCreateWithFlags (virDomainPtr domain, unsigned int flags) +{ + int rv = -1; + remote_domain_create_with_flags_args args; + remote_domain_create_with_flags_ret ret; + struct private_data *priv = domain->conn->privateData; + + remoteDriverLock(priv); + + make_nonnull_domain (&args.dom, domain); + args.flags = flags; + + memset (&ret, 0, sizeof ret); + if (call (domain->conn, priv, 0, REMOTE_PROC_DOMAIN_CREATE_WITH_FLAGS, + (xdrproc_t) xdr_remote_domain_create_with_flags_args, + (char *) &args, + (xdrproc_t) xdr_remote_domain_create_with_flags_ret, + (char *) &ret) == -1) + goto done; + + domain->id = ret.dom.id; + xdr_free ((xdrproc_t) &xdr_remote_domain_create_with_flags_ret, + (char *) &ret); + + rv = 0; + +done: + remoteDriverUnlock(priv); + return rv; +} + static virDomainPtr remoteDomainDefineXML (virConnectPtr conn, const char *xml) { @@ -10215,7 +10247,7 @@ static virDriver remote_driver = { remoteListDefinedDomains, /* listDefinedDomains */ remoteNumOfDefinedDomains, /* numOfDefinedDomains */ remoteDomainCreate, /* domainCreate */ - NULL, /* domainCreateWithFlags */ + remoteDomainCreateWithFlags, /* domainCreateWithFlags */ remoteDomainDefineXML, /* domainDefineXML */ remoteDomainUndefine, /* domainUndefine */ remoteDomainAttachDevice, /* domainAttachDevice */ diff --git a/src/remote/remote_protocol.c b/src/remote/remote_protocol.c index 972bf52..1746317 100644 --- a/src/remote/remote_protocol.c +++ b/src/remote/remote_protocol.c @@ -1203,6 +1203,26 @@ xdr_remote_domain_create_args (XDR *xdrs, remote_domain_create_args *objp) } bool_t +xdr_remote_domain_create_with_flags_args (XDR *xdrs, remote_domain_create_with_flags_args *objp) +{ + + if (!xdr_remote_nonnull_domain (xdrs, &objp->dom)) + return FALSE; + if (!xdr_int (xdrs, &objp->flags)) + return FALSE; + return TRUE; +} + +bool_t +xdr_remote_domain_create_with_flags_ret (XDR *xdrs, remote_domain_create_with_flags_ret *objp) +{ + + if (!xdr_remote_nonnull_domain (xdrs, &objp->dom)) + return FALSE; + return TRUE; +} + +bool_t xdr_remote_domain_define_xml_args (XDR *xdrs, remote_domain_define_xml_args *objp) { diff --git a/src/remote/remote_protocol.h b/src/remote/remote_protocol.h index a600af6..e1f1360 100644 --- a/src/remote/remote_protocol.h +++ b/src/remote/remote_protocol.h @@ -666,6 +666,17 @@ struct remote_domain_create_args { }; typedef struct remote_domain_create_args remote_domain_create_args; +struct remote_domain_create_with_flags_args { + remote_nonnull_domain dom; + int flags; +}; +typedef struct remote_domain_create_with_flags_args remote_domain_create_with_flags_args; + +struct remote_domain_create_with_flags_ret { + remote_nonnull_domain dom; +}; +typedef struct remote_domain_create_with_flags_ret remote_domain_create_with_flags_ret; + struct remote_domain_define_xml_args { remote_nonnull_string xml; }; @@ -2215,6 +2226,7 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_SNAPSHOT_DELETE = 193, REMOTE_PROC_DOMAIN_GET_BLOCK_INFO = 194, REMOTE_PROC_DOMAIN_EVENT_IO_ERROR_REASON = 195, + REMOTE_PROC_DOMAIN_CREATE_WITH_FLAGS = 196, }; typedef enum remote_procedure remote_procedure; @@ -2345,6 +2357,8 @@ extern bool_t xdr_remote_list_defined_domains_args (XDR *, remote_list_defined_ extern bool_t xdr_remote_list_defined_domains_ret (XDR *, remote_list_defined_domains_ret*); extern bool_t xdr_remote_num_of_defined_domains_ret (XDR *, remote_num_of_defined_domains_ret*); extern bool_t xdr_remote_domain_create_args (XDR *, remote_domain_create_args*); +extern bool_t xdr_remote_domain_create_with_flags_args (XDR *, remote_domain_create_with_flags_args*); +extern bool_t xdr_remote_domain_create_with_flags_ret (XDR *, remote_domain_create_with_flags_ret*); extern bool_t xdr_remote_domain_define_xml_args (XDR *, remote_domain_define_xml_args*); extern bool_t xdr_remote_domain_define_xml_ret (XDR *, remote_domain_define_xml_ret*); extern bool_t xdr_remote_domain_undefine_args (XDR *, remote_domain_undefine_args*); @@ -2678,6 +2692,8 @@ extern bool_t xdr_remote_list_defined_domains_args (); extern bool_t xdr_remote_list_defined_domains_ret (); extern bool_t xdr_remote_num_of_defined_domains_ret (); extern bool_t xdr_remote_domain_create_args (); +extern bool_t xdr_remote_domain_create_with_flags_args (); +extern bool_t xdr_remote_domain_create_with_flags_ret (); extern bool_t xdr_remote_domain_define_xml_args (); extern bool_t xdr_remote_domain_define_xml_ret (); extern bool_t xdr_remote_domain_undefine_args (); diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 1ce488c..d1414af 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -3,7 +3,7 @@ * remote_internal driver and libvirtd. This protocol is * internal and may change at any time. * - * Copyright (C) 2006-2008 Red Hat, Inc. + * Copyright (C) 2006-2010 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -696,6 +696,15 @@ struct remote_domain_create_args { remote_nonnull_domain dom; }; +struct remote_domain_create_with_flags_args { + remote_nonnull_domain dom; + int flags; +}; + +struct remote_domain_create_with_flags_ret { + remote_nonnull_domain dom; +}; + struct remote_domain_define_xml_args { remote_nonnull_string xml; }; @@ -2004,7 +2013,8 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_REVERT_TO_SNAPSHOT = 192, REMOTE_PROC_DOMAIN_SNAPSHOT_DELETE = 193, REMOTE_PROC_DOMAIN_GET_BLOCK_INFO = 194, - REMOTE_PROC_DOMAIN_EVENT_IO_ERROR_REASON = 195 + REMOTE_PROC_DOMAIN_EVENT_IO_ERROR_REASON = 195, + REMOTE_PROC_DOMAIN_CREATE_WITH_FLAGS = 196 /* * Notice how the entries are grouped in sets of 10 ? diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index c8f81f3..2e67931 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -406,6 +406,13 @@ struct remote_num_of_defined_domains_ret { struct remote_domain_create_args { remote_nonnull_domain dom; }; +struct remote_domain_create_with_flags_args { + remote_nonnull_domain dom; + int flags; +}; +struct remote_domain_create_with_flags_ret { + remote_nonnull_domain dom; +}; struct remote_domain_define_xml_args { remote_nonnull_string xml; }; -- 1.7.0.1
On Thu, Jun 10, 2010 at 11:16:10AM -0600, Eric Blake wrote:
Define the wire format for the new virDomainCreateWithFlags API, and implement client and server side of marshaling code.
* daemon/remote.c (remoteDispatchDomainCreateWithFlags): Add server side dispatch for virDomainCreateWithFlags. * src/remote/remote_driver.c (remoteDomainCreateWithFlags) (remote_driver): Client side serialization. * src/remote/remote_protocol.x (remote_domain_create_with_flags_args) (remote_domain_create_with_flags_ret) (REMOTE_PROC_DOMAIN_CREATE_WITH_FLAGS): Define wire format. * daemon/remote_dispatch_args.h: Regenerate. * daemon/remote_dispatch_prototypes.h: Likewise. * daemon/remote_dispatch_table.h: Likewise. * src/remote/remote_protocol.c: Likewise. * src/remote/remote_protocol.h: Likewise. * src/remote_protocol-structs: Likewise. diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 1ce488c..d1414af 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -3,7 +3,7 @@ * remote_internal driver and libvirtd. This protocol is * internal and may change at any time. * - * Copyright (C) 2006-2008 Red Hat, Inc. + * Copyright (C) 2006-2010 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -696,6 +696,15 @@ struct remote_domain_create_args { remote_nonnull_domain dom; };
+struct remote_domain_create_with_flags_args { + remote_nonnull_domain dom; + int flags; +};
This should be 'unsigned flags' to match the API header Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
* src/esx/esx_driver.c (esxDomainCreate): Move guts... (esxDomainCreateWithFlags): ...to new function. (esxDriver): Trivially support the new API. * src/lxc/lxc_driver.c (lxcDomainStart, lxcDomainStartWithFlags) (lxcDriver): Likewise. * src/opennebula/one_driver.c (oneDomainStart) (oneDomainStartWithFlags, oneDriver): Likewise. * src/openvz/openvz_driver.c (openvzDomainCreate) (openvzDomainCreateWithFlags, openvzDriver): Likewise. * src/qemu/qemu_driver.c (qemudDomainStart) (qemudDomainStartWithFlags, qemuDriver): Likewise. * src/test/test_driver.c (testDomainCreate) (testDomainCreateWithFlags, testDriver): Likewise. * src/uml/uml_driver.c (umlDomainStart, umlDomainStartWithFlags) (umlDriver): Likewise. * src/vbox/vbox_tmpl.c (vboxDomainCreate) (vboxDomainCreateWithFlags, Driver): Likewise. * src/xen/xen_driver.c (xenUnifiedDomainCreate) (xenUnifiedDomainCreateWithFlags, xenUnifiedDriver): Likewise. * src/xenapi/xenapi_driver.c (xenapiDomainCreate) (xenapiDomainCreateWithFlags, xenapiDriver): Likewise. --- I found the spots to patch by: git grep 'omainCreate \*/' but I chose not to touch the few instances of domainCreate functions tied to xenUnifiedDriver, instead making xen_driver.c reject a non-zero flag before drilling down to the lower-layer create. src/esx/esx_driver.c | 12 +++++++++--- src/lxc/lxc_driver.c | 22 +++++++++++++++++++--- src/opennebula/one_driver.c | 11 +++++++++-- src/openvz/openvz_driver.c | 12 ++++++++++-- src/qemu/qemu_driver.c | 15 +++++++++++++-- src/test/test_driver.c | 10 ++++++++-- src/uml/uml_driver.c | 9 +++++++-- src/vbox/vbox_tmpl.c | 10 ++++++++-- src/xen/xen_driver.c | 12 ++++++++++-- src/xenapi/xenapi_driver.c | 21 ++++++++++++++++++--- 10 files changed, 111 insertions(+), 23 deletions(-) diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 0b2a3b6..1968537 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -2345,7 +2345,7 @@ esxNumberOfDefinedDomains(virConnectPtr conn) static int -esxDomainCreate(virDomainPtr domain) +esxDomainCreateWithFlags(virDomainPtr domain, unsigned int flags) { int result = -1; esxPrivate *priv = domain->conn->privateData; @@ -2355,6 +2355,8 @@ esxDomainCreate(virDomainPtr domain) esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; + virCheckFlags(0, -1); + if (esxVI_EnsureSession(priv->host) < 0) { return -1; } @@ -2397,7 +2399,11 @@ esxDomainCreate(virDomainPtr domain) return result; } - +static int +esxDomainCreate(virDomainPtr domain) +{ + return esxDomainCreateWithFlags(domain, 0); +} static virDomainPtr esxDomainDefineXML(virConnectPtr conn, const char *xml ATTRIBUTE_UNUSED) @@ -3694,7 +3700,7 @@ static virDriver esxDriver = { esxListDefinedDomains, /* listDefinedDomains */ esxNumberOfDefinedDomains, /* numOfDefinedDomains */ esxDomainCreate, /* domainCreate */ - NULL, /* domainCreateWithFlags */ + esxDomainCreateWithFlags, /* domainCreateWithFlags */ esxDomainDefineXML, /* domainDefineXML */ esxDomainUndefine, /* domainUndefine */ NULL, /* domainAttachDevice */ diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 2027abf..19d4dcb 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -1349,20 +1349,23 @@ cleanup: } /** - * lxcDomainStart: + * lxcDomainStartWithFlags: * @dom: domain to start + * @flags: Must be 0 for now * * Looks up domain and starts it. * * Returns 0 on success or -1 in case of error */ -static int lxcDomainStart(virDomainPtr dom) +static int lxcDomainStartWithFlags(virDomainPtr dom, unsigned int flags) { lxc_driver_t *driver = dom->conn->privateData; virDomainObjPtr vm; virDomainEventPtr event = NULL; int ret = -1; + virCheckFlags(0, -1); + lxcDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); if (!vm) { @@ -1402,6 +1405,19 @@ cleanup: } /** + * lxcDomainStart: + * @dom: domain to start + * + * Looks up domain and starts it. + * + * Returns 0 on success or -1 in case of error + */ +static int lxcDomainStart(virDomainPtr dom) +{ + return lxcDomainStartWithFlags(dom, 0); +} + +/** * lxcDomainCreateAndStart: * @conn: pointer to connection * @xml: XML definition of domain @@ -2557,7 +2573,7 @@ static virDriver lxcDriver = { lxcListDefinedDomains, /* listDefinedDomains */ lxcNumDefinedDomains, /* numOfDefinedDomains */ lxcDomainStart, /* domainCreate */ - NULL, /* domainCreateWithFlags */ + lxcDomainStartWithFlags, /* domainCreateWithFlags */ lxcDomainDefine, /* domainDefineXML */ lxcDomainUndefine, /* domainUndefine */ NULL, /* domainAttachDevice */ diff --git a/src/opennebula/one_driver.c b/src/opennebula/one_driver.c index caa0d67..9d7b415 100644 --- a/src/opennebula/one_driver.c +++ b/src/opennebula/one_driver.c @@ -402,7 +402,7 @@ cleanup: return ret; } -static int oneDomainStart(virDomainPtr dom) +static int oneDomainStartWithFlags(virDomainPtr dom, unsigned int flags) { virConnectPtr conn = dom->conn; one_driver_t *driver = conn->privateData; @@ -410,6 +410,8 @@ static int oneDomainStart(virDomainPtr dom) int ret = -1; int oneid; + virCheckFlags(0, -1); + oneDriverLock(driver); vm = virDomainFindByName(&driver->domains, dom->name); @@ -434,6 +436,11 @@ return_point: return ret; } +static int oneDomainStart(virDomainPtr dom) +{ + return oneDomainStartWithFlags(dom, 0); +} + static virDomainPtr oneDomainCreateAndStart(virConnectPtr conn, const char *xml, @@ -755,7 +762,7 @@ static virDriver oneDriver = { oneListDefinedDomains, /* listDefinedDomains */ oneNumDefinedDomains, /* numOfDefinedDomains */ oneDomainStart, /* domainCreate */ - NULL, /* domainCreateWithFlags */ + oneDomainStartWithFlags, /* domainCreateWithFlags */ oneDomainDefine, /* domainDefineXML */ oneDomainUndefine, /* domainUndefine */ NULL, /* domainAttachDevice */ diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index 8ee9ad5..f7da1be 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -958,13 +958,15 @@ cleanup: } static int -openvzDomainCreate(virDomainPtr dom) +openvzDomainCreateWithFlags(virDomainPtr dom, unsigned int flags) { struct openvz_driver *driver = dom->conn->privateData; virDomainObjPtr vm; const char *prog[] = {VZCTL, "--quiet", "start", PROGRAM_SENTINAL, NULL }; int ret = -1; + virCheckFlags(0, -1); + openvzDriverLock(driver); vm = virDomainFindByName(&driver->domains, dom->name); openvzDriverUnlock(driver); @@ -1000,6 +1002,12 @@ cleanup: } static int +openvzDomainCreate(virDomainPtr dom) +{ + return openvzDomainCreateWithFlags(dom, 0); +} + +static int openvzDomainUndefine(virDomainPtr dom) { struct openvz_driver *driver = dom->conn->privateData; @@ -1507,7 +1515,7 @@ static virDriver openvzDriver = { openvzListDefinedDomains, /* listDefinedDomains */ openvzNumDefinedDomains, /* numOfDefinedDomains */ openvzDomainCreate, /* domainCreate */ - NULL, /* domainCreateWithFlags */ + openvzDomainCreateWithFlags, /* domainCreateWithFlags */ openvzDomainDefineXML, /* domainDefineXML */ openvzDomainUndefine, /* domainUndefine */ NULL, /* domainAttachDevice */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 760a27a..13a36ee 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6721,11 +6721,16 @@ cleanup: return ret; } -static int qemudDomainStart(virDomainPtr dom) { +static int +qemudDomainStartWithFlags(virDomainPtr dom, unsigned int flags) +{ struct qemud_driver *driver = dom->conn->privateData; virDomainObjPtr vm; int ret = -1; + /* XXX: Support VIR_DOMAIN_START_PAUSED */ + virCheckFlags(0, -1); + qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); @@ -6760,6 +6765,12 @@ cleanup: } static int +qemudDomainStart(virDomainPtr dom) +{ + return qemudDomainStartWithFlags(dom, 0); +} + +static int qemudCanonicalizeMachineFromInfo(virDomainDefPtr def, virCapsGuestDomainInfoPtr info, char **canonical) @@ -12195,7 +12206,7 @@ static virDriver qemuDriver = { qemudListDefinedDomains, /* listDefinedDomains */ qemudNumDefinedDomains, /* numOfDefinedDomains */ qemudDomainStart, /* domainCreate */ - NULL, /* domainCreateWithFlags */ + qemudDomainStartWithFlags, /* domainCreateWithFlags */ qemudDomainDefine, /* domainDefineXML */ qemudDomainUndefine, /* domainUndefine */ qemudDomainAttachDevice, /* domainAttachDevice */ diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 13eb5e0..5b6f47e 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -2350,12 +2350,14 @@ cleanup: } -static int testDomainCreate(virDomainPtr domain) { +static int testDomainCreateWithFlags(virDomainPtr domain, unsigned int flags) { testConnPtr privconn = domain->conn->privateData; virDomainObjPtr privdom; virDomainEventPtr event = NULL; int ret = -1; + virCheckFlags(0, -1); + testDriverLock(privconn); privdom = virDomainFindByName(&privconn->domains, domain->name); @@ -2389,6 +2391,10 @@ cleanup: return ret; } +static int testDomainCreate(virDomainPtr domain) { + return testDomainCreateWithFlags(domain, 0); +} + static int testDomainUndefine(virDomainPtr domain) { testConnPtr privconn = domain->conn->privateData; virDomainObjPtr privdom; @@ -5261,7 +5267,7 @@ static virDriver testDriver = { testListDefinedDomains, /* listDefinedDomains */ testNumOfDefinedDomains, /* numOfDefinedDomains */ testDomainCreate, /* domainCreate */ - NULL, /* domainCreateWithFlags */ + testDomainCreateWithFlags, /* domainCreateWithFlags */ testDomainDefineXML, /* domainDefineXML */ testDomainUndefine, /* domainUndefine */ NULL, /* domainAttachDevice */ diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 4978e48..49a6e00 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -1576,11 +1576,13 @@ static int umlNumDefinedDomains(virConnectPtr conn) { } -static int umlDomainStart(virDomainPtr dom) { +static int umlDomainStartWithFlags(virDomainPtr dom, unsigned int flags) { struct uml_driver *driver = dom->conn->privateData; virDomainObjPtr vm; int ret = -1; + virCheckFlags(0, -1); + umlDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); @@ -1599,6 +1601,9 @@ cleanup: return ret; } +static int umlDomainStart(virDomainPtr dom) { + return umlDomainStartWithFlags(dom, 0); +} static virDomainPtr umlDomainDefine(virConnectPtr conn, const char *xml) { struct uml_driver *driver = conn->privateData; @@ -1892,7 +1897,7 @@ static virDriver umlDriver = { umlListDefinedDomains, /* listDefinedDomains */ umlNumDefinedDomains, /* numOfDefinedDomains */ umlDomainStart, /* domainCreate */ - NULL, /* domainCreateWithFlags */ + umlDomainStartWithFlags, /* domainCreateWithFlags */ umlDomainDefine, /* domainDefineXML */ umlDomainUndefine, /* domainUndefine */ NULL, /* domainAttachDevice */ diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index 3d94ce9..542310e 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -3139,7 +3139,7 @@ cleanup: return ret; } -static int vboxDomainCreate(virDomainPtr dom) { +static int vboxDomainCreateWithFlags(virDomainPtr dom, unsigned int flags) { VBOX_OBJECT_CHECK(dom->conn, int, -1); IMachine **machines = NULL; IProgress *progress = NULL; @@ -3151,6 +3151,8 @@ static int vboxDomainCreate(virDomainPtr dom) { nsresult rc; int i = 0; + virCheckFlags(0, -1); + if (!dom->name) { vboxError(VIR_ERR_INTERNAL_ERROR, "%s", _("Error while reading the domain name")); @@ -3357,6 +3359,10 @@ cleanup: return ret; } +static int vboxDomainCreate(virDomainPtr dom) { + return vboxDomainCreateWithFlags(dom, 0); +} + static virDomainPtr vboxDomainDefineXML(virConnectPtr conn, const char *xml) { VBOX_OBJECT_CHECK(conn, virDomainPtr, NULL); IMachine *machine = NULL; @@ -8173,7 +8179,7 @@ virDriver NAME(Driver) = { vboxListDefinedDomains, /* listDefinedDomains */ vboxNumOfDefinedDomains, /* numOfDefinedDomains */ vboxDomainCreate, /* domainCreate */ - NULL, /* domainCreateWithFlags */ + vboxDomainCreateWithFlags, /* domainCreateWithFlags */ vboxDomainDefineXML, /* domainDefineXML */ vboxDomainUndefine, /* domainUndefine */ vboxDomainAttachDevice, /* domainAttachDevice */ diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index ca6b246..2fd52ba 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -1381,11 +1381,13 @@ xenUnifiedNumOfDefinedDomains (virConnectPtr conn) } static int -xenUnifiedDomainCreate (virDomainPtr dom) +xenUnifiedDomainCreateWithFlags (virDomainPtr dom, unsigned int flags) { GET_PRIVATE(dom->conn); int i; + virCheckFlags(0, -1); + for (i = 0; i < XEN_UNIFIED_NR_DRIVERS; ++i) if (priv->opened[i] && drivers[i]->domainCreate && drivers[i]->domainCreate (dom) == 0) @@ -1394,6 +1396,12 @@ xenUnifiedDomainCreate (virDomainPtr dom) return -1; } +static int +xenUnifiedDomainCreate (virDomainPtr dom) +{ + return xenUnifiedDomainCreateWithFlags(dom, 0); +} + static virDomainPtr xenUnifiedDomainDefineXML (virConnectPtr conn, const char *xml) { @@ -1941,7 +1949,7 @@ static virDriver xenUnifiedDriver = { xenUnifiedListDefinedDomains, /* listDefinedDomains */ xenUnifiedNumOfDefinedDomains, /* numOfDefinedDomains */ xenUnifiedDomainCreate, /* domainCreate */ - NULL, /* domainCreateWithFlags */ + xenUnifiedDomainCreateWithFlags, /* domainCreateWithFlags */ xenUnifiedDomainDefineXML, /* domainDefineXML */ xenUnifiedDomainUndefine, /* domainUndefine */ xenUnifiedDomainAttachDevice, /* domainAttachDevice */ diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c index 518c4a7..cefcf3b 100644 --- a/src/xenapi/xenapi_driver.c +++ b/src/xenapi/xenapi_driver.c @@ -1448,17 +1448,20 @@ xenapiNumOfDefinedDomains (virConnectPtr conn) } /* - * xenapiDomainCreate + * xenapiDomainCreateWithFlags * * starts a VM * Return 0 on success or -1 in case of error */ static int -xenapiDomainCreate (virDomainPtr dom) +xenapiDomainCreateWithFlags (virDomainPtr dom, unsigned int flags) { xen_vm_set *vms; xen_vm vm; xen_session *session = ((struct _xenapiPrivate *)(dom->conn->privateData))->session; + + virCheckFlags(0, -1); + if (xen_vm_get_by_name_label(session, &vms, dom->name) && vms->size > 0) { if (vms->size != 1) { xenapiSessionErrorHandler(dom->conn, VIR_ERR_INTERNAL_ERROR, @@ -1482,6 +1485,18 @@ xenapiDomainCreate (virDomainPtr dom) } /* + * xenapiDomainCreate + * + * starts a VM + * Return 0 on success or -1 in case of error + */ +static int +xenapiDomainCreate (virDomainPtr dom) +{ + return xenapiDomainCreateWithFlags(dom, 0); +} + +/* * xenapiDomainDefineXML * * Defines a domain from the given XML but does not start it @@ -1744,7 +1759,7 @@ static virDriver xenapiDriver = { xenapiListDefinedDomains, /* listDefinedDomains */ xenapiNumOfDefinedDomains, /* numOfDefinedDomains */ xenapiDomainCreate, /* domainCreate */ - NULL, /* domainCreateWithFlags */ + xenapiDomainCreateWithFlags, /* domainCreateWithFlags */ xenapiDomainDefineXML, /* domainDefineXML */ xenapiDomainUndefine, /* domainUndefine */ NULL, /* domainAttachDevice */ -- 1.7.0.1
On Thu, Jun 10, 2010 at 11:16:11AM -0600, Eric Blake wrote:
* src/esx/esx_driver.c (esxDomainCreate): Move guts... (esxDomainCreateWithFlags): ...to new function. (esxDriver): Trivially support the new API. * src/lxc/lxc_driver.c (lxcDomainStart, lxcDomainStartWithFlags) (lxcDriver): Likewise. * src/opennebula/one_driver.c (oneDomainStart) (oneDomainStartWithFlags, oneDriver): Likewise. * src/openvz/openvz_driver.c (openvzDomainCreate) (openvzDomainCreateWithFlags, openvzDriver): Likewise. * src/qemu/qemu_driver.c (qemudDomainStart) (qemudDomainStartWithFlags, qemuDriver): Likewise. * src/test/test_driver.c (testDomainCreate) (testDomainCreateWithFlags, testDriver): Likewise. * src/uml/uml_driver.c (umlDomainStart, umlDomainStartWithFlags) (umlDriver): Likewise. * src/vbox/vbox_tmpl.c (vboxDomainCreate) (vboxDomainCreateWithFlags, Driver): Likewise. * src/xen/xen_driver.c (xenUnifiedDomainCreate) (xenUnifiedDomainCreateWithFlags, xenUnifiedDriver): Likewise. * src/xenapi/xenapi_driver.c (xenapiDomainCreate) (xenapiDomainCreateWithFlags, xenapiDriver): Likewise. ---
I found the spots to patch by: git grep 'omainCreate \*/'
but I chose not to touch the few instances of domainCreate functions tied to xenUnifiedDriver, instead making xen_driver.c reject a non-zero flag before drilling down to the lower-layer create.
src/esx/esx_driver.c | 12 +++++++++--- src/lxc/lxc_driver.c | 22 +++++++++++++++++++--- src/opennebula/one_driver.c | 11 +++++++++-- src/openvz/openvz_driver.c | 12 ++++++++++-- src/qemu/qemu_driver.c | 15 +++++++++++++-- src/test/test_driver.c | 10 ++++++++-- src/uml/uml_driver.c | 9 +++++++-- src/vbox/vbox_tmpl.c | 10 ++++++++-- src/xen/xen_driver.c | 12 ++++++++++-- src/xenapi/xenapi_driver.c | 21 ++++++++++++++++++--- 10 files changed, 111 insertions(+), 23 deletions(-)
ACK Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
Match earlier change for qemu pause support with virDomainCreateXML. * src/qemu/qemu_driver.c (qemudDomainObjStart): Add parameter; all callers changed. (qemudDomainStartWithFlags): Implement flag support. --- Question: should qemudDomainObjStart take an 'unsigned int flags' instead of 'bool start_paused', in case we want to support further flags in the future? src/qemu/qemu_driver.c | 16 +++++++++------- 1 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 13a36ee..df04ea1 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -148,7 +148,8 @@ static void qemuDomainEventQueue(struct qemud_driver *driver, static int qemudDomainObjStart(virConnectPtr conn, struct qemud_driver *driver, - virDomainObjPtr vm); + virDomainObjPtr vm, + bool start_paused); static int qemudStartVMDaemon(virConnectPtr conn, struct qemud_driver *driver, @@ -643,7 +644,7 @@ qemuAutostartDomain(void *payload, const char *name ATTRIBUTE_UNUSED, void *opaq } else { if (vm->autostart && !virDomainObjIsActive(vm) && - qemudDomainObjStart(data->conn, data->driver, vm) < 0) { + qemudDomainObjStart(data->conn, data->driver, vm, false) < 0) { err = virGetLastError(); VIR_ERROR(_("Failed to autostart VM '%s': %s"), vm->def->name, @@ -6685,7 +6686,8 @@ static int qemudNumDefinedDomains(virConnectPtr conn) { static int qemudDomainObjStart(virConnectPtr conn, struct qemud_driver *driver, - virDomainObjPtr vm) + virDomainObjPtr vm, + bool start_paused) { int ret = -1; char *managed_save; @@ -6706,7 +6708,7 @@ static int qemudDomainObjStart(virConnectPtr conn, goto cleanup; } - ret = qemudStartVMDaemon(conn, driver, vm, NULL, false, -1, NULL); + ret = qemudStartVMDaemon(conn, driver, vm, NULL, start_paused, -1, NULL); if (ret != -1) { virDomainEventPtr event = virDomainEventNewFromObj(vm, @@ -6728,8 +6730,7 @@ qemudDomainStartWithFlags(virDomainPtr dom, unsigned int flags) virDomainObjPtr vm; int ret = -1; - /* XXX: Support VIR_DOMAIN_START_PAUSED */ - virCheckFlags(0, -1); + virCheckFlags(VIR_DOMAIN_START_PAUSED, -1); qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); @@ -6751,7 +6752,8 @@ qemudDomainStartWithFlags(virDomainPtr dom, unsigned int flags) goto endjob; } - ret = qemudDomainObjStart(dom->conn, driver, vm); + ret = qemudDomainObjStart(dom->conn, driver, vm, + (flags & VIR_DOMAIN_START_PAUSED) != 0); endjob: if (qemuDomainObjEndJob(vm) == 0) -- 1.7.0.1
On Thu, Jun 10, 2010 at 11:16:12AM -0600, Eric Blake wrote:
Match earlier change for qemu pause support with virDomainCreateXML.
* src/qemu/qemu_driver.c (qemudDomainObjStart): Add parameter; all callers changed. (qemudDomainStartWithFlags): Implement flag support. ---
Question: should qemudDomainObjStart take an 'unsigned int flags' instead of 'bool start_paused', in case we want to support further flags in the future?
src/qemu/qemu_driver.c | 16 +++++++++------- 1 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 13a36ee..df04ea1 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -148,7 +148,8 @@ static void qemuDomainEventQueue(struct qemud_driver *driver,
static int qemudDomainObjStart(virConnectPtr conn, struct qemud_driver *driver, - virDomainObjPtr vm); + virDomainObjPtr vm, + bool start_paused);
static int qemudStartVMDaemon(virConnectPtr conn, struct qemud_driver *driver, @@ -643,7 +644,7 @@ qemuAutostartDomain(void *payload, const char *name ATTRIBUTE_UNUSED, void *opaq } else { if (vm->autostart && !virDomainObjIsActive(vm) && - qemudDomainObjStart(data->conn, data->driver, vm) < 0) { + qemudDomainObjStart(data->conn, data->driver, vm, false) < 0) { err = virGetLastError(); VIR_ERROR(_("Failed to autostart VM '%s': %s"), vm->def->name, @@ -6685,7 +6686,8 @@ static int qemudNumDefinedDomains(virConnectPtr conn) {
static int qemudDomainObjStart(virConnectPtr conn, struct qemud_driver *driver, - virDomainObjPtr vm) + virDomainObjPtr vm, + bool start_paused) { int ret = -1; char *managed_save; @@ -6706,7 +6708,7 @@ static int qemudDomainObjStart(virConnectPtr conn, goto cleanup; }
- ret = qemudStartVMDaemon(conn, driver, vm, NULL, false, -1, NULL); + ret = qemudStartVMDaemon(conn, driver, vm, NULL, start_paused, -1, NULL); if (ret != -1) { virDomainEventPtr event = virDomainEventNewFromObj(vm, @@ -6728,8 +6730,7 @@ qemudDomainStartWithFlags(virDomainPtr dom, unsigned int flags) virDomainObjPtr vm; int ret = -1;
- /* XXX: Support VIR_DOMAIN_START_PAUSED */ - virCheckFlags(0, -1); + virCheckFlags(VIR_DOMAIN_START_PAUSED, -1);
qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); @@ -6751,7 +6752,8 @@ qemudDomainStartWithFlags(virDomainPtr dom, unsigned int flags) goto endjob; }
- ret = qemudDomainObjStart(dom->conn, driver, vm); + ret = qemudDomainObjStart(dom->conn, driver, vm, + (flags & VIR_DOMAIN_START_PAUSED) != 0);
endjob: if (qemuDomainObjEndJob(vm) == 0)
ACK Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
Make 'start --paused' mirror 'create --paused'. * tools/virsh.c (cmdStart): Try new virDomainCreateWithFlags API first. * tools/virsh.pod (start): Document --paused. --- tools/virsh.c | 9 ++++++++- tools/virsh.pod | 7 +++++-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index d60c27b..b208cae 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1306,6 +1306,7 @@ static const vshCmdOptDef opts_start[] = { #ifndef WIN32 {"console", VSH_OT_BOOL, 0, N_("attach to console after creation")}, #endif + {"paused", VSH_OT_BOOL, 0, N_("leave the guest paused after creation")}, {NULL, 0, 0, NULL} }; @@ -1317,6 +1318,7 @@ cmdStart(vshControl *ctl, const vshCmd *cmd) #ifndef WIN32 int console = vshCommandOptBool(cmd, "console"); #endif + unsigned int flags = VIR_DOMAIN_NONE; if (!vshConnectionUsability(ctl, ctl->conn, TRUE)) return FALSE; @@ -1330,7 +1332,12 @@ cmdStart(vshControl *ctl, const vshCmd *cmd) return FALSE; } - if (virDomainCreate(dom) == 0) { + if (vshCommandOptBool(cmd, "paused")) + flags |= VIR_DOMAIN_START_PAUSED; + + /* Try newer API first, but fall back to older one if possible. */ + if (virDomainCreateWithFlags(dom, flags) == 0 || + (flags == 0 && virDomainCreate(dom) == 0)) { vshPrint(ctl, _("Domain %s started\n"), virDomainGetName(dom)); #ifndef WIN32 diff --git a/tools/virsh.pod b/tools/virsh.pod index b54003c..f5b44be 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -469,9 +469,12 @@ services must be shutdown in the domain. The exact behavior of a domain when it shuts down is set by the I<on_shutdown> parameter in the domain's XML definition. -=item B<start> I<domain-name> +=item B<start> I<domain-name> optional I<--console> I<--paused> -Start a (previously defined) inactive domain. +Start a (previously defined) inactive domain. The domain will be paused +if the I<--paused> option is used and supported by the driver; otherwise +it will be running. +If I<--console> is requested, attach to the console after creation. =item B<suspend> I<domain-id> -- 1.7.0.1
On Thu, Jun 10, 2010 at 11:16:13AM -0600, Eric Blake wrote:
Make 'start --paused' mirror 'create --paused'.
* tools/virsh.c (cmdStart): Try new virDomainCreateWithFlags API first. * tools/virsh.pod (start): Document --paused. --- tools/virsh.c | 9 ++++++++- tools/virsh.pod | 7 +++++-- 2 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c index d60c27b..b208cae 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1306,6 +1306,7 @@ static const vshCmdOptDef opts_start[] = { #ifndef WIN32 {"console", VSH_OT_BOOL, 0, N_("attach to console after creation")}, #endif + {"paused", VSH_OT_BOOL, 0, N_("leave the guest paused after creation")}, {NULL, 0, 0, NULL} };
@@ -1317,6 +1318,7 @@ cmdStart(vshControl *ctl, const vshCmd *cmd) #ifndef WIN32 int console = vshCommandOptBool(cmd, "console"); #endif + unsigned int flags = VIR_DOMAIN_NONE;
if (!vshConnectionUsability(ctl, ctl->conn, TRUE)) return FALSE; @@ -1330,7 +1332,12 @@ cmdStart(vshControl *ctl, const vshCmd *cmd) return FALSE; }
- if (virDomainCreate(dom) == 0) { + if (vshCommandOptBool(cmd, "paused")) + flags |= VIR_DOMAIN_START_PAUSED; + + /* Try newer API first, but fall back to older one if possible. */ + if (virDomainCreateWithFlags(dom, flags) == 0 || + (flags == 0 && virDomainCreate(dom) == 0)) { vshPrint(ctl, _("Domain %s started\n"), virDomainGetName(dom));
This should really be if (flags) virDomainCreateWithFlags(dom, flags) else virDomainCreate(dom); avoiding the need for any try-and-fallback behaviour in the case where flags=0 Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
* src/remote/remote_driver.c (remoteDomainCreate): Rewrite to take 1 RPC instead of 2 when talking to newer server, at expense of 3 RPC instead of 2 for older server. --- I had already started coding this, but then Daniel Berrange reminded me off-list that changing existing API just for a minor optimization in the number of RPC calls is not a good idea: clients (like virsh) are already free to do the optimization themselves, and libvirt RPC calls tend not to be the critical path in the system in the first place. So, I'm posting this to have it archived, before nuking the commit from my repo. src/remote/remote_driver.c | 20 ++++++++++++++++++++ 1 files changed, 20 insertions(+), 0 deletions(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 7052bf1..a613ead 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -3001,6 +3001,8 @@ static int remoteDomainCreate (virDomainPtr domain) { int rv = -1; + remote_domain_create_with_flags_args args0; + remote_domain_create_with_flags_ret ret0; remote_domain_create_args args; remote_domain_lookup_by_uuid_args args2; remote_domain_lookup_by_uuid_ret ret2; @@ -3008,6 +3010,24 @@ remoteDomainCreate (virDomainPtr domain) remoteDriverLock(priv); + /* First, try the newer API, for fewer call()s. */ + make_nonnull_domain (&args0.dom, domain); + args0.flags = 0; + + memset (&ret0, 0, sizeof ret0); + if (call (domain->conn, priv, 0, REMOTE_PROC_DOMAIN_CREATE_WITH_FLAGS, + (xdrproc_t) xdr_remote_domain_create_with_flags_args, + (char *) &args0, + (xdrproc_t) xdr_remote_domain_create_with_flags_ret, + (char *) &ret0) == 0) { + domain->id = ret0.dom.id; + xdr_free ((xdrproc_t) &xdr_remote_domain_create_with_flags_ret, + (char *) &ret0); + rv = 0; + goto done; + } + + /* Fall back to the older 2-call sequence. */ make_nonnull_domain (&args.dom, domain); if (call (domain->conn, priv, 0, REMOTE_PROC_DOMAIN_CREATE, -- 1.7.0.1
On Thu, Jun 10, 2010 at 11:16:14AM -0600, Eric Blake wrote:
* src/remote/remote_driver.c (remoteDomainCreate): Rewrite to take 1 RPC instead of 2 when talking to newer server, at expense of 3 RPC instead of 2 for older server. ---
I had already started coding this, but then Daniel Berrange reminded me off-list that changing existing API just for a minor optimization in the number of RPC calls is not a good idea: clients (like virsh) are already free to do the optimization themselves, and libvirt RPC calls tend not to be the critical path in the system in the first place.
So, I'm posting this to have it archived, before nuking the commit from my repo.
src/remote/remote_driver.c | 20 ++++++++++++++++++++ 1 files changed, 20 insertions(+), 0 deletions(-)
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 7052bf1..a613ead 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -3001,6 +3001,8 @@ static int remoteDomainCreate (virDomainPtr domain) { int rv = -1; + remote_domain_create_with_flags_args args0; + remote_domain_create_with_flags_ret ret0; remote_domain_create_args args; remote_domain_lookup_by_uuid_args args2; remote_domain_lookup_by_uuid_ret ret2; @@ -3008,6 +3010,24 @@ remoteDomainCreate (virDomainPtr domain)
remoteDriverLock(priv);
+ /* First, try the newer API, for fewer call()s. */ + make_nonnull_domain (&args0.dom, domain); + args0.flags = 0; + + memset (&ret0, 0, sizeof ret0); + if (call (domain->conn, priv, 0, REMOTE_PROC_DOMAIN_CREATE_WITH_FLAGS, + (xdrproc_t) xdr_remote_domain_create_with_flags_args, + (char *) &args0, + (xdrproc_t) xdr_remote_domain_create_with_flags_ret, + (char *) &ret0) == 0) { + domain->id = ret0.dom.id; + xdr_free ((xdrproc_t) &xdr_remote_domain_create_with_flags_ret, + (char *) &ret0); + rv = 0; + goto done; + } + + /* Fall back to the older 2-call sequence. */ make_nonnull_domain (&args.dom, domain);
if (call (domain->conn, priv, 0, REMOTE_PROC_DOMAIN_CREATE,
NACK, this is not required. The existing method is guarenteed to always exist, so there is no benefit to calling the new method & it adds an extra RPC call when talking to old servers Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
participants (3)
-
Daniel P. Berrange -
Eric Blake -
Jiri Denemark