Re: [libvirt] [PATCH v1 1/2] Backcompt for console devices in virDomainDeviceInfoIterate
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. On 2012年08月08日 22:52, Li Zhang wrote:
Histrically, the first <console> element is treated as the 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 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 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@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; device.data.chr = def->consoles[i]; if (cb(def, &device, &def->consoles[i]->info, opaque) < 0) return -1;
-- Li Zhang IBM China Linux Technology Centre
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@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@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (2)
-
Eric Blake -
Li Zhang