Re: [libvirt] [PATCH v2 1/1] Backcompt for console devices in virDomainDeviceInfoIterate

On 09/12/2012 10:32 AM, Osier Yang wrote: > On 2012年09月12日 16:06, Martin Kletzander wrote: >> On 09/11/2012 04:57 AM, 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 >>> 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. >>> >>> 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> >>> >>> Console configuration is default, 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 as two different devices, and in the >>> qemuAssignSpaprVIOAddress(), >>> it will compare these two devices' addressed. If they have same address, >>> it will report address confilict error. >> >> s/confilict/conflict/ >> >>> >>> 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. >>> >> >> I'm wondering if this is the place we want to fix it. If somebody wants >> to use the iterate function for something else, they must accept that >> the fact that the first serial console will be skipped. OTOH it is >> common (and the only supported way) to have the serial console. >> >>> Signed-off-by: Li Zhang&lt;zhlcindy(a)linux.vnet.ibm.com&gt; >>> --- >>> * v1 -> v2: >>> Rebase v1 on latest version of libvirt. >>> >>> 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 8952b69..0e71b06 100644 >>> --- a/src/conf/domain_conf.c >>> +++ b/src/conf/domain_conf.c >>> @@ -2054,6 +2054,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; >>> >> >> I was thinkinf about fixing it in the callback function, configuration >> parser and so on, but this looks like the best solution, so ACK from me, >> is it OK if I push this with the address in Cc? > > It's the one listed in AUTHORS. So should be fine. > That's why I asked, because the one from 'From:' is not there. OK than, pushed with fixes, thanks both of you ;)

>> >> Martin >> >> -- >> libvir-list mailing list >> libvir-list(a)redhat.com >> https://www.redhat.com/mailman/listinfo/libvir-list >