[libvirt] [PATCH] qemu: Restored original console alias
Because of some crazy backward compatibility, console device is in some cases just an alias to a serial device. This means, in the process of generating XML description of a domain, all the interesting info is taken from corresponding serial device, if that's the case. Including the device alias. That means, we produce: <console type='pty' tty='/dev/pts/20'> ... <alias name='serial0'/> </console> (notice the assigned alias) Maybe this is okay, maybe its wrong either. Anyway, later, when libvirtd restarts, and we parse the state XML file, we read the wrong alias back. Hence, the internal representation is different to the state it was in prior the libvirtd restart. --- src/qemu/qemu_domain.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 8d79066..96d88ec 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -804,6 +804,34 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, dev->data.chr->source.data.nix.listen = true; } + /* For some really crazy back compat in virDomainDefFormatInternal we must + * restore the original console alias. For hvm domains, we are formatting + * a dummy console device (based on a serial device which it refers to) + * instead of the original one. That means the device aliases in memory + * and in the formatted XML are not in sync. While in memory we still have + * 'consoleN', in the formatted XML we have 'serialN'. */ + if (dev->type == VIR_DOMAIN_DEVICE_CHR && + dev->data.chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE && + dev->data.chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL && + STREQ(def->os.type, "hvm")) { + int id; + char *alias = dev->data.chr->info.alias; + const char *serial_alias = "serial"; + + if (alias && STRPREFIX(alias, serial_alias)) { + alias += strlen(serial_alias); + + if (virStrToLong_i(alias, NULL, 10, &id) < 0) + goto cleanup; + + VIR_FREE(dev->data.chr->info.alias); + if (virAsprintf(&dev->data.chr->info.alias, "console%d", id) < 0) { + virReportOOMError(); + goto cleanup; + } + } + } + ret = 0; cleanup: -- 1.8.1.5
On Fri, Jun 28, 2013 at 01:11:40PM +0200, Michal Privoznik wrote:
Because of some crazy backward compatibility, console device is in some cases just an alias to a serial device. This means, in the process of generating XML description of a domain, all the interesting info is taken from corresponding serial device, if that's the case. Including the device alias. That means, we produce: <console type='pty' tty='/dev/pts/20'> ... <alias name='serial0'/> </console>
(notice the assigned alias)
Maybe this is okay, maybe its wrong either. Anyway, later, when libvirtd restarts, and we parse the state XML file, we read the wrong alias back. Hence, the internal representation is different to the state it was in prior the libvirtd restart. --- src/qemu/qemu_domain.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 8d79066..96d88ec 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -804,6 +804,34 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, dev->data.chr->source.data.nix.listen = true; }
+ /* For some really crazy back compat in virDomainDefFormatInternal we must + * restore the original console alias. For hvm domains, we are formatting + * a dummy console device (based on a serial device which it refers to) + * instead of the original one. That means the device aliases in memory + * and in the formatted XML are not in sync. While in memory we still have + * 'consoleN', in the formatted XML we have 'serialN'. */ + if (dev->type == VIR_DOMAIN_DEVICE_CHR && + dev->data.chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE && + dev->data.chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL && + STREQ(def->os.type, "hvm")) { + int id; + char *alias = dev->data.chr->info.alias; + const char *serial_alias = "serial"; + + if (alias && STRPREFIX(alias, serial_alias)) { + alias += strlen(serial_alias); + + if (virStrToLong_i(alias, NULL, 10, &id) < 0) + goto cleanup; + + VIR_FREE(dev->data.chr->info.alias); + if (virAsprintf(&dev->data.chr->info.alias, "console%d", id) < 0) {
The <alias> names in the XML must match the 'id' values used for the devices created in QEMU. So if the <console> device is a clone of the primary serial device, then it is not correct to be changing 'serial0' to 'console0' here. Dnaiel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On Fri, Jun 28, 2013 at 01:11:40PM +0200, Michal Privoznik wrote:
Because of some crazy backward compatibility, console device is in some cases just an alias to a serial device. This means, in the process of generating XML description of a domain, all the interesting info is taken from corresponding serial device, if that's the case. Including the device alias. That means, we produce: <console type='pty' tty='/dev/pts/20'> ... <alias name='serial0'/> </console>
(notice the assigned alias)
Maybe this is okay, maybe its wrong either. Anyway, later, when libvirtd restarts, and we parse the state XML file, we read the wrong alias back. Hence, the internal representation is different to the state it was in prior the libvirtd restart. --- src/qemu/qemu_domain.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 8d79066..96d88ec 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -804,6 +804,34 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, dev->data.chr->source.data.nix.listen = true; }
+ /* For some really crazy back compat in virDomainDefFormatInternal we must + * restore the original console alias. For hvm domains, we are formatting + * a dummy console device (based on a serial device which it refers to) + * instead of the original one. That means the device aliases in memory + * and in the formatted XML are not in sync. While in memory we still have + * 'consoleN', in the formatted XML we have 'serialN'. */ + if (dev->type == VIR_DOMAIN_DEVICE_CHR && + dev->data.chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE && + dev->data.chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL && + STREQ(def->os.type, "hvm")) { + int id; + char *alias = dev->data.chr->info.alias; + const char *serial_alias = "serial"; + + if (alias && STRPREFIX(alias, serial_alias)) { + alias += strlen(serial_alias); + + if (virStrToLong_i(alias, NULL, 10, &id) < 0) + goto cleanup; + + VIR_FREE(dev->data.chr->info.alias); + if (virAsprintf(&dev->data.chr->info.alias, "console%d", id) < 0) { + virReportOOMError(); + goto cleanup; + } + } + } + ret = 0;
cleanup:
Ok, the situation we have is - The VM is configured with a <serial> port - The <serial> port config is copied to <console> for purposes of back compatibility - After starting the guest QEMU has a device with id==serial0 while virDomainDefPtr has def->seriales[0]->info == "serial0" def->consoles[0]->info == "console0" and the XML shown to the user, and saved to the state XML is <serial type='pty'> <source path='/dev/pts/7'/> <target port='0'/> <alias name='serial0'/> </serial> <console type='pty' tty='/dev/pts/7'> <source path='/dev/pts/7'/> <target type='serial' port='0'/> <alias name='serial0'/> </console> This inconsistency/inaccuracy in the virDomainDefPtr is bad - After re-starting libvirtd, while the guest is runing def->seriales[0]->info == "serial0" def->consoles[0]->info == "serial0" because it got this data from the state XML file and the XML shown to the user remains: <serial type='pty'> <source path='/dev/pts/7'/> <target port='0'/> <alias name='serial0'/> </serial> <console type='pty' tty='/dev/pts/7'> <source path='/dev/pts/7'/> <target type='serial' port='0'/> <alias name='serial0'/> </console> So the inconsistency has been fixed at this point IIUC, this patch is intended to change things so that after libvirtd is restarted, we get: def->seriales[0]->info == "serial0" def->consoles[0]->info == "console0" but this is fixing the wrong thing. There is only one physical device emulated in the guest, which is a serial port with id==serial0, and this is reflected correctly in the XML we generate. Only the internal struct is different. So what needs fixing is the code which populated def->consoles[0]->info with "console0" instead of the correct "serial0" string at VM startup. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (2)
-
Daniel P. Berrange -
Michal Privoznik