[libvirt] [PATCH] Don't validate disk type in virsh attach-disk

Jim Meyering jim at meyering.net
Mon Mar 2 17:15:08 UTC 2009


Daniel P. Berrange wrote:
...
>> Changing how the test driver handles this case
>> won't give a better diagnostic for all the other drivers.
>
> At the same time though you don't want to fill virsh up with a pile of
> different error reporting logic for each & every driver. The fine grained
> error reporting needs to be in the drivers themselves, so all users of
> the API get correct error messages, and not just virsh. As such having
> it in virsh is at best duplicating information, or at worst just giving
> wrong information.

We agree on that:
virsh's UI should give good diagnostics, and those
should be derivable from the results returned by the API.

While that's fine in the long run, it's a shame to see a short-term
regression (worse diagnostic) while we wait for all of the drivers to
perform this check.  Actually, it seems like each driver could expose
which types it accepts (or provide a validType predicate), and then
virsh could use that.

Cole,
Note that if you do drop the existing test, you'll have to make
an additional change.  Otherwise, an invalid "type" could hose the
resulting XML.  You'd have to xml-quote that string before adding it to
the XML you eventually pass to virDomainAttachDevice.




More information about the libvir-list mailing list