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

[Martin Kletzander]

On Tue, Sep 29, 2015 at 05:03:07PM +0200, Michal Privoznik wrote:
>On 25.09.2015 14:45, Martin Kletzander wrote:
>> 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.
>
>Yes and no. qemuDomainAssignAddresses() expects to see vm->def which
>already has the device attached. Moreover, it not only works over all
>devices in the domain (filling in all the missing addresses), but
>possibly creating new controllers too (e.g. plugging new pci bridges if
>running out of addresses on a bus).
>
>Does anybody have a bright idea how this can be fixed apart from obvious
>one - breaking whole address assignment code into parts and reassembling
>it back together again?
>

Unfortunately, not.  Well, maybe the address allocation I was trying
to push as GSoC and/or Outreachy project could've fixed that, but
nobody was able to finish that, so we don't know =)

>Michal
-------------- 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/20150929/68efc1ce/attachment-0001.sig>