[libvirt] [PATCH 3/4] virsh: New command cmdChangeMedia

Eric Blake eblake at redhat.com
Mon Feb 13 22:18:43 UTC 2012


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   eblake at redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120213/1b206578/attachment-0001.sig>


More information about the libvir-list mailing list