[libvirt] [PATCH] qemuDomainAttachDeviceLive: Check provided disk address

[Martin Kletzander]

On Fri, Sep 25, 2015 at 06:41:44AM -0400, John Ferlan wrote:
>
>
>On 09/25/2015 05:38 AM, Michal Privoznik wrote:
>> On 25.09.2015 11:36, Martin Kletzander wrote:
>>> On Thu, Sep 24, 2015 at 05:43:08PM +0200, Michal Privoznik wrote:
>>>> https://bugzilla.redhat.com/show_bug.cgi?id=1257844
>>>>
>>>> Imagine an user who is trying to attach a disk to a domain with
>>>> the following XML:
>>>>
>>>>  <disk type='block' device='disk'>
>>>>    <driver name='qemu' type='raw'/>
>>>>    <source dev='/dev/sr0'/>
>>>>    <target dev='vde' bus='virtio'/>
>>>>    <address type='drive' controller='0' bus='0' target='0' unit='0'/>
>>>>  </disk>
>>>>
>>>> The XML is obviously wrong. It's trying to attach a virtio disk
>>>> onto non-PCI bus. We should forbid that.
>>>>
>>>> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>>>> ---
>>>> src/qemu/qemu_hotplug.c | 7 +++++++
>>>> 1 file changed, 7 insertions(+)
>>>>
>>>
>>> How come this is not handled in qemuDomainAssignAddresses(), it
>>> doesn't get called?  There's a check for exactly that in
>>> qemuAssignDevicePCISlots().
>>
>> Exactly! qemuAssignDevicePCISlots() is called only in case of --config.
>>
>
>Seems to me this may be more of a generic problem - a user providing the
>wrong address type for the type of device. I have a recollection of

Yes, and since we have checks for those, it's confusing to me why
would qemuAssignDevicePCISlots() be called only with --config.  Can we
use that code which checks for more things already?  For example, the
here-missing virtio-mmio.

>discussing this while having a patch series reviewed in the last month
>or two. Still searching for that conversation - I thought it was during
>my series with SCSI hostdev and disk address assignment conflicts, but
>that was geared more towards two user address supplied disks could have
>the same address and we don't check that. It may also have been during
>the similar CCW/s390 series where a ccw/s390 address was used when it
>shouldn't have been (bug 1258361), which is more alike this bug.
>
>John
>
>--
>libvir-list mailing list
>libvir-list at redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20150925/98971f64/attachment-0001.sig>