[libvirt] [RFC PATCH 1/4] libvirt: add VIR_DOMAIN_START_PERSISTENT flag

Added a VIR_DOMAIN_START_PERSISTENT flag for virDomainCreateXML() so that the guest remains defined after it is destroyed. The result of using this flag is equivilent to calling virDomainDefineXML() followed by virDomainCreate() or virDomainCreateWithFlags(). --- Not sure if this is the correct place to add "Since 1.0.3 (likely 1.0.4)" --- include/libvirt/libvirt.h.in | 1 + src/libvirt.c | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index ad30cd0..e119215 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -336,6 +336,7 @@ typedef enum { VIR_DOMAIN_START_AUTODESTROY = 1 << 1, /* Automatically kill guest when virConnectPtr is closed */ VIR_DOMAIN_START_BYPASS_CACHE = 1 << 2, /* Avoid file system cache pollution */ VIR_DOMAIN_START_FORCE_BOOT = 1 << 3, /* Boot, discarding any managed save */ + VIR_DOMAIN_START_PERSISTENT = 1 << 4, /* Define guest to exist after it is destroyed */ } virDomainCreateFlags; diff --git a/src/libvirt.c b/src/libvirt.c index 1e78500..abe59cd 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -1951,6 +1951,10 @@ virDomainGetConnect(virDomainPtr dom) * is destroyed, or if the host is restarted (see virDomainDefineXML() to * define persistent domains). * + * If the VIR_DOMAIN_START_PERSISTENT flag is set, the guest domain + * will remain defined even after the guest is destroyed. This is + * similar to if you had called virDomainDefineXML() and virDomainCreate(). + * * If the VIR_DOMAIN_START_PAUSED flag is set, the guest domain * will be started, but its CPUs will remain paused. The CPUs * can later be manually started using virDomainResume. -- 1.7.12.4

Adds support to the qemu driver for the VIR_DOMAIN_START_PERSISTENT flag --- Figured this would be better to "define" the domain before starting it, rather than starting and then defining but I can change as people prefer. --- src/qemu/qemu_driver.c | 45 +++++++++++++++++++++++++++++++++++---------- 1 file changed, 35 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 76b2800..0a3bbc2 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1448,14 +1448,17 @@ static virDomainPtr qemuDomainCreate(virConnectPtr conn, const char *xml, virDomainDefPtr def = NULL; virDomainObjPtr vm = NULL; virDomainPtr dom = NULL; - virDomainEventPtr event = NULL; - virDomainEventPtr event2 = NULL; + virDomainEventPtr event_define = NULL; + virDomainEventPtr event_start = NULL; + virDomainEventPtr event_pause = NULL; unsigned int start_flags = VIR_QEMU_PROCESS_START_COLD; virQEMUCapsPtr qemuCaps = NULL; virCapsPtr caps = NULL; + virQEMUDriverConfigPtr cfg = NULL; virCheckFlags(VIR_DOMAIN_START_PAUSED | - VIR_DOMAIN_START_AUTODESTROY, NULL); + VIR_DOMAIN_START_AUTODESTROY | + VIR_DOMAIN_START_PERSISTENT, NULL); if (flags & VIR_DOMAIN_START_PAUSED) start_flags |= VIR_QEMU_PROCESS_START_PAUSED; @@ -1491,6 +1494,24 @@ static virDomainPtr qemuDomainCreate(virConnectPtr conn, const char *xml, def = NULL; + /* Attempt to make the domain persistent before we start it */ + if (flags & VIR_DOMAIN_START_PERSISTENT) { + vm->persistent = 1; + + cfg = virQEMUDriverGetConfig(driver); + + if (virDomainSaveConfig(cfg->configDir, vm->newDef) < 0) { + VIR_INFO("Deleting domain '%s'", vm->def->name); + qemuDomainRemoveInactive(driver, vm); + vm = NULL; + goto cleanup; + } + + event_define = virDomainEventNewFromObj(vm, + VIR_DOMAIN_EVENT_DEFINED, + VIR_DOMAIN_EVENT_DEFINED_ADDED); + } + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; /* XXXX free the 'vm' we created ? */ @@ -1504,21 +1525,22 @@ static virDomainPtr qemuDomainCreate(virConnectPtr conn, const char *xml, goto cleanup; } - event = virDomainEventNewFromObj(vm, + event_start = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_STARTED, VIR_DOMAIN_EVENT_STARTED_BOOTED); - if (event && (flags & VIR_DOMAIN_START_PAUSED)) { + if (event_start && (flags & VIR_DOMAIN_START_PAUSED)) { /* There are two classes of event-watching clients - those * that only care about on/off (and must see a started event * no matter what, but don't care about suspend events), and * those that also care about running/paused. To satisfy both * client types, we have to send two events. */ - event2 = virDomainEventNewFromObj(vm, + event_pause = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_SUSPENDED, VIR_DOMAIN_EVENT_SUSPENDED_PAUSED); } virDomainAuditStart(vm, "booted", true); + VIR_INFO("Creating domain '%s'", vm->def->name); dom = virGetDomain(conn, vm->def->name, vm->def->uuid); if (dom) dom->id = vm->def->id; @@ -1530,13 +1552,16 @@ cleanup: virDomainDefFree(def); if (vm) virObjectUnlock(vm); - if (event) { - qemuDomainEventQueue(driver, event); - if (event2) - qemuDomainEventQueue(driver, event2); + if (event_define) + qemuDomainEventQueue(driver, event_define); + if (event_start) { + qemuDomainEventQueue(driver, event_start); + if (event_pause) + qemuDomainEventQueue(driver, event_pause); } virObjectUnref(caps); virObjectUnref(qemuCaps); + virObjectUnref(cfg); return dom; } -- 1.7.12.4

Add support for the persistent flag to virsh create to allow creating of guests that remain defined after they are stopped. --- The discussion on https://www.redhat.com/archives/libvir-list/2013-January/msg00490.html was more inclined to have 'virsh create --persistent' so this adds that. --- tools/virsh-domain.c | 7 +++++++ tools/virsh.pod | 5 ++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 3e4be89..cb56b3d 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6414,6 +6414,11 @@ static const vshCmdOptDef opts_create[] = { .flags = 0, .help = N_("automatically destroy the guest when virsh disconnects") }, + {.name = "persistent", + .type = VSH_OT_BOOL, + .flags = 0, + .help = N_("keep the guest persistently defined") + }, {.name = NULL} }; @@ -6439,6 +6444,8 @@ cmdCreate(vshControl *ctl, const vshCmd *cmd) flags |= VIR_DOMAIN_START_PAUSED; if (vshCommandOptBool(cmd, "autodestroy")) flags |= VIR_DOMAIN_START_AUTODESTROY; + if (vshCommandOptBool(cmd, "persistent")) + flags |= VIR_DOMAIN_START_PERSISTENT; dom = virDomainCreateXML(ctl->conn, buffer, flags); VIR_FREE(buffer); diff --git a/tools/virsh.pod b/tools/virsh.pod index 96666c4..3ea4ba6 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -539,6 +539,7 @@ the I<--force> flag may be specified, requesting to disconnect any existing sessions, such as in a case of a broken connection. =item B<create> I<FILE> [I<--console>] [I<--paused>] [I<--autodestroy>] +[I<--persistent>] 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 @@ -547,7 +548,9 @@ is used and supported by the driver; otherwise it will be running. If I<--console> is requested, attach to the console after creation. If I<--autodestroy> is requested, then the guest will be automatically destroyed when virsh closes its connection to libvirt, or otherwise -exits. +exits. If I<--persistent> is requested, keep the domain defined after +it is destroyed. The behavior is similar to having issued the B<define> +command followed by the B<start> command. B<Example> -- 1.7.12.4

Added support for a --start flag to the define command --- Implements https://www.redhat.com/archives/libvir-list/2013-January/msg00490.html in a new way using the persistent flag for create. --- tools/virsh-domain.c | 9 ++++++++- tools/virsh.pod | 5 +++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index cb56b3d..5a18316 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6475,6 +6475,9 @@ static const vshCmdInfo info_define[] = { {.name = "desc", .data = N_("Define a domain.") }, + {.name = "starT", + .data = N_("Start domain after defining it.") + }, {.name = NULL} }; @@ -6501,7 +6504,11 @@ cmdDefine(vshControl *ctl, const vshCmd *cmd) if (virFileReadAll(from, VSH_MAX_XML_FILE, &buffer) < 0) return false; - dom = virDomainDefineXML(ctl->conn, buffer); + if (vshCommandOptBool(cmd, "start")) + dom = virDomainCreateXML(ctl->conn, buffer, + VIR_DOMAIN_START_PERSISTENT); + else + dom = virDomainDefineXML(ctl->conn, buffer); VIR_FREE(buffer); if (dom != NULL) { diff --git a/tools/virsh.pod b/tools/virsh.pod index 3ea4ba6..91bbd6c 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -558,11 +558,12 @@ B<Example> vi domain.xml (or make changes with your other text editor) virsh create domain.xml -=item B<define> I<FILE> +=item B<define> I<FILE> [I<--start>] Define a domain from an XML <file>. The domain definition is registered but not started. If domain is already running, the changes will take -effect on the next boot. +effect on the next boot. If I<--start> is requested, start the domain +after defining it. =item B<desc> I<domain> [[I<--live>] [I<--config>] | [I<--current>]] [I<--title>] [I<--edit>] [I<--new-desc> -- 1.7.12.4

On Fri, Feb 22, 2013 at 11:40 AM, Doug Goldstein <cardoe@cardoe.com> wrote:
Added support for a --start flag to the define command ---
Implements https://www.redhat.com/archives/libvir-list/2013-January/msg00490.html in a new way using the persistent flag for create.
--- tools/virsh-domain.c | 9 ++++++++- tools/virsh.pod | 5 +++-- 2 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index cb56b3d..5a18316 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6475,6 +6475,9 @@ static const vshCmdInfo info_define[] = { {.name = "desc", .data = N_("Define a domain.") }, + {.name = "starT", + .data = N_("Start domain after defining it.") + }, {.name = NULL} };
@@ -6501,7 +6504,11 @@ cmdDefine(vshControl *ctl, const vshCmd *cmd) if (virFileReadAll(from, VSH_MAX_XML_FILE, &buffer) < 0) return false;
- dom = virDomainDefineXML(ctl->conn, buffer); + if (vshCommandOptBool(cmd, "start")) + dom = virDomainCreateXML(ctl->conn, buffer, + VIR_DOMAIN_START_PERSISTENT); + else + dom = virDomainDefineXML(ctl->conn, buffer); VIR_FREE(buffer);
if (dom != NULL) { diff --git a/tools/virsh.pod b/tools/virsh.pod index 3ea4ba6..91bbd6c 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -558,11 +558,12 @@ B<Example> vi domain.xml (or make changes with your other text editor) virsh create domain.xml
-=item B<define> I<FILE> +=item B<define> I<FILE> [I<--start>]
Define a domain from an XML <file>. The domain definition is registered but not started. If domain is already running, the changes will take -effect on the next boot. +effect on the next boot. If I<--start> is requested, start the domain +after defining it.
=item B<desc> I<domain> [[I<--live>] [I<--config>] | [I<--current>]] [I<--title>] [I<--edit>] [I<--new-desc> -- 1.7.12.4
This should have been a RFC patch. Not a real ready for integration patch. -- Doug Goldstein

On Fri, Feb 22, 2013 at 11:40:32AM -0600, Doug Goldstein wrote:
Added a VIR_DOMAIN_START_PERSISTENT flag for virDomainCreateXML() so that the guest remains defined after it is destroyed. The result of using this flag is equivilent to calling virDomainDefineXML() followed by virDomainCreate() or virDomainCreateWithFlags(). ---
Not sure if this is the correct place to add "Since 1.0.3 (likely 1.0.4)"
--- include/libvirt/libvirt.h.in | 1 + src/libvirt.c | 4 ++++ 2 files changed, 5 insertions(+)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index ad30cd0..e119215 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -336,6 +336,7 @@ typedef enum { VIR_DOMAIN_START_AUTODESTROY = 1 << 1, /* Automatically kill guest when virConnectPtr is closed */ VIR_DOMAIN_START_BYPASS_CACHE = 1 << 2, /* Avoid file system cache pollution */ VIR_DOMAIN_START_FORCE_BOOT = 1 << 3, /* Boot, discarding any managed save */ + VIR_DOMAIN_START_PERSISTENT = 1 << 4, /* Define guest to exist after it is destroyed */ } virDomainCreateFlags;
As previously discussed, I'm against including this in the API because it doesn't offer anything that can't already be done with the existing APIs. Indeed this is worse than the existing APIs because this has only been wired up for the QEMU driver and none others. It also increases the size of the code and thus maintenance work for each driver for no feature gain. By all means add the flags to virsh, but they can be done using the existing APIs IMHO. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On Fri, Feb 22, 2013 at 11:48 AM, Daniel P. Berrange <berrange@redhat.com> wrote:
On Fri, Feb 22, 2013 at 11:40:32AM -0600, Doug Goldstein wrote:
Added a VIR_DOMAIN_START_PERSISTENT flag for virDomainCreateXML() so that the guest remains defined after it is destroyed. The result of using this flag is equivilent to calling virDomainDefineXML() followed by virDomainCreate() or virDomainCreateWithFlags(). ---
Not sure if this is the correct place to add "Since 1.0.3 (likely 1.0.4)"
--- include/libvirt/libvirt.h.in | 1 + src/libvirt.c | 4 ++++ 2 files changed, 5 insertions(+)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index ad30cd0..e119215 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -336,6 +336,7 @@ typedef enum { VIR_DOMAIN_START_AUTODESTROY = 1 << 1, /* Automatically kill guest when virConnectPtr is closed */ VIR_DOMAIN_START_BYPASS_CACHE = 1 << 2, /* Avoid file system cache pollution */ VIR_DOMAIN_START_FORCE_BOOT = 1 << 3, /* Boot, discarding any managed save */ + VIR_DOMAIN_START_PERSISTENT = 1 << 4, /* Define guest to exist after it is destroyed */ } virDomainCreateFlags;
As previously discussed, I'm against including this in the API because it doesn't offer anything that can't already be done with the existing APIs. Indeed this is worse than the existing APIs because this has only been wired up for the QEMU driver and none others. It also increases the size of the code and thus maintenance work for each driver for no feature gain.
By all means add the flags to virsh, but they can be done using the existing APIs IMHO.
Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
So I'm happy with any way that's accepted. I implemented it this way based on Eric's feedback in https://www.redhat.com/archives/libvir-list/2013-January/msg00554.html I was really just looking for a friendly way to do: $ virsh define --start domain.xml I can stick with my current solution if that's preferred, which is to have a bash function: function virshdefstart() { virsh define $1 && virsh start $1 } -- Doug Goldstein
participants (2)
-
Daniel P. Berrange
-
Doug Goldstein