[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 09/13] qemu: use controller object alias in commandline for virtio-serial device




On 05/12/2015 11:16 AM, Laine Stump wrote:
> On 05/08/2015 08:05 PM, John Ferlan wrote:
>>
>> On 05/05/2015 02:03 PM, Laine Stump wrote:
>>> The commandline generator for a virtio-serial device had a hardcoded
>>> printf of "virtio-serial%d" as the id of the virtio-serial
>>> controller. This patch changes it to use the alias of the controller
>>> instead. Because the function that finds a controller alias requires a
>>> pointer to the domainDef in order to get the list of controllers, the
>>> arglist of a few functions had to have this added.
>>>
>>> Once this was done, the literal string QEMU_VIRTIO_SERIAL_PREFIX was
>>> no longer needed, so it has been removed.
>>> ---
>>>  src/qemu/qemu_command.c | 28 +++++++++++++++++-----------
>>>  src/qemu/qemu_command.h |  1 -
>>>  2 files changed, 17 insertions(+), 12 deletions(-)
>>>
>> ACK - although a couple of notes (a cscope search on "bus=" finds :
>>
>> qemuUSBId
>>
>>  - where there are decision points to use usb or usb%d
> 
> Yeah. It looks like that is another exception (so another place where
> the status XML isn't correctly reflecting the id used in qemu).
> 
> 
>> qemuBuildSCSIHostdevDevStr
>>
>>  - where bus=scsi%d is used
>>
> 
> 
> Yes, this one should be changed too.
> 
> Also, there is a place in qemuBuildDeviceAddressStr() where it is making
> decisions about the name to use for the PCI bus based on whether or not
> PCI_MULTIBUS is true. Instead, the function that builds the alias for
> controllers should create the correct name for the one-and-only PCI
> controller in this case, and qemuBuildDeviceAddressStr() should just
> unconditionally use that name.
> 
> Should I merge all of these into a single patch? Or a separate patch for
> each?
> 
> 

I'm fine with a single patch being submitted - although perhaps "for
review purposes" - combine 7-9 as one... then post the new areas with
the mindset to merge the patches before pushing. That way it makes it
easier to review/explain the new changes.

John


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]