Re: [libvirt] [PATCH] qemu: Restored original console alias

2013-07-02 Thread Daniel P. Berrange 
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 :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] qemu: Restored original console alias

2013-06-28 Thread Michal Privoznik 
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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] qemu: Restored original console alias

2013-06-28 Thread Daniel P. Berrange 
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 :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list