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 at redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/