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

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(a)redhat.com&gt; >>>> --- >>>> 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.