[libvirt] Consultation of the patch "util: Prepare helpers for unpriv_sgio setting"

[Eric Blake]

On 03/18/2013 08:13 AM, Osier Yang wrote:
>>>>>> I added a disk to virtio-scsi bus through the VM's XML file like
>>>>>> this:
>>>>>> <disk type='block' device='disk'>
>>>>>> <driver name='qemu' type='raw' io='threads'/>
>>>>>> <source dev='/mnt/test.raw'/>
>>>>>> <target dev='sdb' bus='scsi'/>
>>>>>> <shareable/>
>>>>>> </disk>
>>>>>>
>>>>>> Then when I started the VM, I met the error.
>>>>>> [root at build SOURCES]# virsh start pan64ga
>>>>>> error: Failed to start domain pan64ga
>>>>>> error: Unable to get minor number of device '/mnt/test.raw': Invalid
>>>>>> argument

>>> It's not reasonable. As the disk source you use is /mnt/test.raw, I
>>> believe it's not a block device. However, you used disk type='block'.
>>> Libvirt honors what you specified, and tries to grab the device IDs
>>> of the disk source (it expects the disk source is block, but it's not).
>>
>> This inspires me thinking of we should validate the disk source type
>> somewhere (when parsing or in the driver, as it's general for all
>> drivers).
>>
> 
> Any opinions on should we validate whether the disk type and the
> specified source matches?
> 
> Just realized that validating when parsing doesn't make sense,
> because the validation can imply the checking of whether the
> disk source existing or not, however, for a inactive domain,
> it's normal that its disk source may not existing.
> 
> So the only way to do the validation is inside driver. But does
> it imply we will have regression if doing so? As yuxh said, the
> guest can actually be started successfully with block disk has
> a plain file source, not sure if it's expected result of qemu
> though.

I think we are safe arguing that most users have not mixed type='block'
with non-block files.  The fact that it previously worked was a bug in
our level of validation; since we did not document that it should work,
I'm okay if we keep the new behavior of erroring out when a file is not
a block device.  A nicer error than 'Unable to get minor number of
device '/mnt/test.raw':' would be appropriate, but I don't think that we
are introducing too much of a regression by rejecting an unusual
combination that was never documented to work.

Still, just to be safe, I'd like to get DV or danpb's opinion.

-- 
Eric Blake   eblake 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: 621 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20130320/2d3d86dd/attachment-0001.sig>