On 08/09/2012 08:07 PM, Li Zhang wrote:
Hi Daniel and Eric,
Would you please help review this patch?
This is one bug fix for pSeries.
x86 doesn't have such a problem, because serial devices has no bus
address on x86.
I think this won't break x86.
Apologies for the delayed review.
Hmm, I couldn't find the original version of this patch; would you mind
sending a v2 rebased to the latest, so that it is easier for me to apply?
On 2012年08月08日 22:52, Li Zhang wrote:
> Histrically, the first <console> element is treated as the
s/Histrically/Historically/
> alias of a <serial> device. In the
virDomainDeviceInfoIterate,
> This situation is not considered. It still handles the first <console>
> element as another devices, which means that for console[0] with
s/devices/device/
> serial targetType, it calls callback function another time.
> It will cause the problem of address conflicts when assigning
> spapr-vio address for serial device on pSeries guest.
>
> The following describes the problem:
>
> For pSeries guest, the serial configuration in the xml file
> is as the following:
> <serial type='pty'>
> <target port='0'/>
> <address type='spapr-vio'/>
> </serial>
>
> There is no console configuration in this file. The dumped xml file
> is as the following:
> <serial type='pty'>
> <source path='/dev/pts/5'/>
> <target port='0'/>
> <alias name='serial0'/>
> <address type='spapr-vio' reg='0x30000000'/>
> </serial>
> <console type='pty' tty='/dev/pts/5'>
> <source path='/dev/pts/5'/>
> <target type='serial' port='0'/>
> <alias name='serial0'/>
> <address type='spapr-vio' reg='0x30000000'/>
> </console>
>
> It shows that the <console> device is the alias of serial device.
> So its address is the same as the serial device. When dectecting
s/dectecting/detecting/
> the conflicts in the qemuAssignSpaprVIOAddress the first console
> and the serial device conflicts because virDomainDeviceInfoIterate()
> still handle these are two different devices, and in the
> qemuAssignSpaprVIOAddress(),
> it will compare these two devices' addressed. If they have same address,
> it will report address error. Actually, they should have the same
> address,
> and the error shouldn't be reported.
>
> So this patch is to handle the first console which targetType is serial
> as the alias of serial device to avoid address conflicts error reported.
>
> Signed-off-by: Li Zhang <zhlcindy(a)linux.vnet.ibm.com>
> ---
> src/conf/domain_conf.c | 3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index d8c0969..cddf6ce 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -2035,6 +2035,9 @@ int virDomainDeviceInfoIterate(virDomainDefPtr def,
> return -1;
> }
> for (i = 0; i < def->nconsoles ; i++) {
> + if ((STREQ(def->os.type, "hvm")) && i == 0 &&
> + def->consoles[i]->targetType ==
> VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL)
> + continue;
Based on the description, this patch seems sane, but since I can't see
the original, I haven't applied it yet.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library
http://libvirt.org