On 02/03/2012 02:23 AM, Osier Yang wrote: > One could use it to eject, insert, or update media of the CDROM > or floppy drive. See the documents for more details. > --- > tools/virsh.c | 134 > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 files changed, 134 insertions(+), 0 deletions(-)
I would squash 3/4 and 4/4 into one patch (new commands should be
committed with documentation).
> +
> + if ((eject && insert) ||
> + (insert && update) ||
> + (eject && update)) {
> + vshError(ctl, "%s", _("--eject, --insert, and --update must be
> specified "
> + "exclusively."));
> + return false;
> + }
> +
> + if (!eject && !insert && !update) {
> + vshError(ctl, "%s", _("One of --eject, --insert, and --update should
> be "
> + "specified"));
> + return false;
> + }
Seems reasonable.
> +
> + if (eject)
> + prepare_flags |= VSH_PREPARE_DISK_XML_EJECT;
> +
> + if (insert)
> + prepare_flags |= VSH_PREPARE_DISK_XML_INSERT;
> +
> + if (update)
> + prepare_flags |= VSH_PREPARE_DISK_XML_UPDATE;
But that means that you are using these as an enum, not as a bitmask to
be OR'd together.
> +
> + if (insert && !source) {
> + vshError(ctl, "%s", _("No disk source specified for inserting"));
> + goto cleanup;
> + }
Shouldn't you also error out if (eject && source)?
> + if (virDomainUpdateDeviceFlags(dom, disk_xml, flags) != 0) {
> + vshError(ctl, "%s", _("Failed to insert media"));
The error message is a bit misleading in the --eject case, since we
didn't insert media, but removed it.
But overall, I think this is a long over-due command.
--
Eric Blake ebl...@redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
