Cole Robinson wrote: > Jim Meyering wrote: >> 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. >> > > The idea here is that virsh isn't the one to decide. It could very well > be that driver foo doesn't support cdrom devices, so even though virsh > lets it through, it's still not valid. Only AttachDevice can give us the > full picture. Sure, I understand that, but if the result of dropping the harmless (once fixed) up-front check is a diagnostic that is less helpful, why drop it? Especially with a 6 or 7-parameter command, it's important to point out which part is causing trouble, whenever possible. The alternative is to give better diagnostics from each and every driver. > Sounds like we should just implement AttachDevice for the test driver: > it could parse the device xml and unconditionally insert it into the Changing how the test driver handles this case won't give a better diagnostic for all the other drivers. > guest config. This would certainly be useful in testing the above virsh > commands (and for testing device attaches in virt-manager, among others -- Libvir-list mailing list Libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list