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

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.

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