Cole Robinson wrote:
> virsh attempts to validate the requested disk type, rather than just let
> the underlying driver do it. This was erroneously denying floppy device
> attaches. Patch attached.
Hi Cole,
Your patch would do the job.
Before:
$ virsh -c test:///default attach-disk 1 s t --type floppy --mode readonly
error: No support floppy in command 'attach-disk'
with the patch:
$ virsh -c test:///default attach-disk 1 s t --type floppy --mode readonly
error: this function is not supported by the hypervisor: virDomainAttachDevice
But consider what happens with e.g., "--type bogus".
Before, it'd mention the invalid type, "bogus".
With the patch, it doesn't (at least not using the test driver).
So maybe it'd be better to keep the up-front type check.
Of course, if we do keep the test in cmdAttachDisk,
the format string should be changed to this:
vshError(ctl, FALSE, _("No %s support in command 'attach-disk'"), type);
> commit 3481061f6ed960269d9b29a4a1380367d557cf37
> Author: Cole Robinson <crobinso@xxxxxxxxxx>
> Date: Fri Feb 27 10:47:49 2009 -0500
>
> Let virt driver validate disk type for us in virsh attach-disk.
>
> diff --git a/src/virsh.c b/src/virsh.c
> index 298dde0..5c9c49f 100644
> --- a/src/virsh.c
> +++ b/src/virsh.c
> @@ -5022,13 +5022,6 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)
> type = vshCommandOptString(cmd, "type", NULL);
> mode = vshCommandOptString(cmd, "mode", NULL);
>
> - if (type) {
> - if (STRNEQ(type, "cdrom") && STRNEQ(type, "disk")) {
> - vshError(ctl, FALSE, _("No support %s in command 'attach-disk'"), type);
> - goto cleanup;
> - }
> - }
> -
> if (driver) {
> if (STREQ(driver, "file") || STREQ(driver, "tap")) {
> isFile = 1;
>
> --
> Libvir-list mailing list
> Libvir-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list