On Mon, Aug 28, 2006 at 12:16:43AM +0100, Daniel P. Berrange wrote:
The attached patch adds new commands to virsh, and modifies the list
command.
* 'list' is given two additional parameters '--inactive' to tell it to
list inactive instances, instead of active instances, and --all to
tell it to list both active & instance domains. NB inactive domains
will show with an ID of -1 and state 'shut off'.
* 'define' defines an inactive domain - instructing the backend to
save the XML definition in some persist location. The domain is
not started.
* 'undefine' removes an inactive domain - the persist config file
managed by the backend is deleted. The domain must not be running
when this is run.
* 'start' starts up an inactive domain which was previously defined
via the 'define' command.
This sounds fine to me !
[...]
+ {"all", VSH_OT_BOOL, 0, "list inactive &
active domains"},
Good idea,, makes the code sligly more complex, but nice to have
+ if (!(dom = vshCommandOptDomain(ctl, cmd, "domain",
&name)))
+ return FALSE;
+
+ if (virDomainUndefine(dom) == 0) {
+ vshPrint(ctl, "Domain %s has been undefined\n", name);
+ } else {
+ vshError(ctl, FALSE, "Failed to undefine domain\n");
<nitpick> Hum, the domain name could be provided in the message </nitpick>
vshError(ctl, FALSE, "Failed to undefine domain %s\n", name);
+/*
+ * "start" command
+ */
+static vshCmdInfo info_start[] = {
+ {"syntax", "start a domain from an XML <file>"},
+ {"help", "start a domain from an XML file"},
+ {"desc", "Start a domain."},
+ {NULL, NULL}
+};
I wonder here about the Xen case. We should soonish have code to read
the /etc/xen config files, it would make some sense to be able to use the
start command to launch those then, unless you really want to force converting
to XML to define. I understand that currently the API allows to define only
based on an XML description, but wouldn't it make sense to automatically
integrate parseable /etc/xen definition files ?
The description also lead to think one should pass an XML instance path,
what about changing those to:
"start a defined but non-running domain" ?
+static vshCmdOptDef opts_start[] = {
+ {"name", VSH_OT_DATA, VSH_OFLAG_REQ, "name of the inactive
domain" },
+ {NULL, 0, 0, NULL}
+};
Could be nice to allow passing UUID too here, names are relatively weak.
+static int
+cmdStart(vshControl * ctl, vshCmd * cmd)
+{
+ virDomainPtr dom;
+ char *name;
+ int found;
+ int ret = TRUE;
+
+ if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ return FALSE;
+
+ name = vshCommandOptString(cmd, "name", &found);
+ if (!found)
+ return FALSE;
just start with virDomainLookupByUUID, and then fallback to that case.
+ dom = virDomainLookupByName(ctl->conn, name);
+ if (!dom)
+ return FALSE;
Looks fine, just push it :-)
Daniel
--
Red Hat Virtualization group
http://redhat.com/virtualization/
Daniel Veillard | virtualization library
http://libvirt.org/
veillard(a)redhat.com | libxml GNOME XML XSLT toolkit
http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine
http://rpmfind.net/