[Libvir] [PATCH] hypervisor version

This is another patch which may not be popular? Xen's extra version does not fit in libvirt's release field (since it's part of an int). Instead of printing out the wrong value, just display major.minor in virsh. Mark

On Thu, Jun 14, 2007 at 05:06:16PM -0400, Mark Johnson wrote:
This is another patch which may not be popular? Xen's extra version does not fit in libvirt's release field (since it's part of an int).
Instead of printing out the wrong value, just display major.minor in virsh.
Hmm, so with Xen we have two backend impls of the Version API, one talking to the hypervisor which only ever returns the first 2 components, and the other talking to XenD which processes all 3. As you say, in practice the extra version from Xen is effectively garbage So while as root I see # virsh version Compiled against library: libvir 0.2.2 Using library: libvir 0.2.2 Using API: Xen 3.0.1 Running hypervisor: Xen 3.1.0 If run as non-root I instead seee $ virsh version Compiled against library: libvir 0.2.2 Using library: libvir 0.2.2 Using API: Xen 3.0.1 Running hypervisor: Xen 3.730.259 I think instead of this patch to change the virsh driver though, we should change teh xend_internal.c file to ignore the extra_version data from XenD as there's no way to meaningfully interpret it as an int. Dan, -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|

On 6/14/07, Daniel P. Berrange <berrange@redhat.com> wrote:
On Thu, Jun 14, 2007 at 05:06:16PM -0400, Mark Johnson wrote:
This is another patch which may not be popular? Xen's extra version does not fit in libvirt's release field (since it's part of an int).
Instead of printing out the wrong value, just display major.minor in virsh.
Hmm, so with Xen we have two backend impls of the Version API, one talking to the hypervisor which only ever returns the first 2 components, and the other talking to XenD which processes all 3.
As you say, in practice the extra version from Xen is effectively garbage
So while as root I see
# virsh version Compiled against library: libvir 0.2.2 Using library: libvir 0.2.2 Using API: Xen 3.0.1 Running hypervisor: Xen 3.1.0
If run as non-root I instead seee
$ virsh version Compiled against library: libvir 0.2.2 Using library: libvir 0.2.2 Using API: Xen 3.0.1 Running hypervisor: Xen 3.730.259
I think instead of this patch to change the virsh driver though, we should change teh xend_internal.c file to ignore the extra_version data from XenD as there's no way to meaningfully interpret it as an int.
ah, I didn't check that one.. Maybe both? 3.1.0 could be incorrect. e.g. it could be 3.1.2. And certainly 3.730.259 is wrong :-) I suppose it could cheat and send a char or two in the rev? Mark.

On Thu, Jun 14, 2007 at 11:27:21PM +0100, Daniel P. Berrange wrote:
On Thu, Jun 14, 2007 at 05:06:16PM -0400, Mark Johnson wrote:
This is another patch which may not be popular? Xen's extra version does not fit in libvirt's release field (since it's part of an int).
Instead of printing out the wrong value, just display major.minor in virsh.
Hmm, so with Xen we have two backend impls of the Version API, one talking to the hypervisor which only ever returns the first 2 components, and the other talking to XenD which processes all 3.
As you say, in practice the extra version from Xen is effectively garbage
So while as root I see
# virsh version Compiled against library: libvir 0.2.2 Using library: libvir 0.2.2 Using API: Xen 3.0.1 Running hypervisor: Xen 3.1.0
If run as non-root I instead seee
$ virsh version Compiled against library: libvir 0.2.2 Using library: libvir 0.2.2 Using API: Xen 3.0.1 Running hypervisor: Xen 3.730.259
I think instead of this patch to change the virsh driver though, we should change teh xend_internal.c file to ignore the extra_version data from XenD as there's no way to meaningfully interpret it as an int.
Agreed, let's fix at the source instead of just dropping the data in the user land tool. I'm more concerned by getting the API right. Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/

Saving the dom0 patch for last :-) On 6/15/07, Daniel Veillard <veillard@redhat.com> wrote:
On Thu, Jun 14, 2007 at 11:27:21PM +0100, Daniel P. Berrange wrote:
On Thu, Jun 14, 2007 at 05:06:16PM -0400, Mark Johnson wrote:
This is another patch which may not be popular? Xen's extra version does not fit in libvirt's release field (since it's part of an int).
Instead of printing out the wrong value, just display major.minor in virsh.
Hmm, so with Xen we have two backend impls of the Version API, one talking to the hypervisor which only ever returns the first 2 components, and the other talking to XenD which processes all 3.
As you say, in practice the extra version from Xen is effectively garbage
So while as root I see
# virsh version Compiled against library: libvir 0.2.2 Using library: libvir 0.2.2 Using API: Xen 3.0.1 Running hypervisor: Xen 3.1.0
If run as non-root I instead seee
$ virsh version Compiled against library: libvir 0.2.2 Using library: libvir 0.2.2 Using API: Xen 3.0.1 Running hypervisor: Xen 3.730.259
I think instead of this patch to change the virsh driver though, we should change teh xend_internal.c file to ignore the extra_version data from XenD as there's no way to meaningfully interpret it as an int.
Agreed, let's fix at the source instead of just dropping the data in the user land tool. I'm more concerned by getting the API right.
I'm not sure where to go from here.. Is the suggestion to change xend_internal/xen_internal to not set rev and remove the display of the hypervisor rev in virsh? Or is it something else? Thanks, Mark

On Fri, Jun 15, 2007 at 06:26:53PM -0400, Mark Johnson wrote:
Saving the dom0 patch for last :-)
On 6/15/07, Daniel Veillard <veillard@redhat.com> wrote:
On Thu, Jun 14, 2007 at 11:27:21PM +0100, Daniel P. Berrange wrote:
On Thu, Jun 14, 2007 at 05:06:16PM -0400, Mark Johnson wrote:
This is another patch which may not be popular? Xen's extra version does not fit in libvirt's release field (since it's part of an int).
Instead of printing out the wrong value, just display major.minor in virsh.
Hmm, so with Xen we have two backend impls of the Version API, one talking to the hypervisor which only ever returns the first 2 components, and the other talking to XenD which processes all 3.
As you say, in practice the extra version from Xen is effectively garbage
So while as root I see
# virsh version Compiled against library: libvir 0.2.2 Using library: libvir 0.2.2 Using API: Xen 3.0.1 Running hypervisor: Xen 3.1.0
If run as non-root I instead seee
$ virsh version Compiled against library: libvir 0.2.2 Using library: libvir 0.2.2 Using API: Xen 3.0.1 Running hypervisor: Xen 3.730.259
I think instead of this patch to change the virsh driver though, we should change teh xend_internal.c file to ignore the extra_version data from XenD as there's no way to meaningfully interpret it as an int.
Agreed, let's fix at the source instead of just dropping the data in the user land tool. I'm more concerned by getting the API right.
I'm not sure where to go from here..
Is the suggestion to change xend_internal/xen_internal to not set rev and remove the display of the hypervisor rev in virsh? Or is it something else?
Well ideally I would prefer xend to not send something crazy for 'release' but since that's unlikely, and since the hypervisor only provide major/minor anyway, let's just drop the release in src/xend_internal.c , I'm suggesting the enclosed patch, Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/

Hi, Daniel I would prefer keep extra in code. (extra region stores additional info) for example (in extra) xen-unstable : -unstable fedora7 :.4-1-1.2898.2.3 Thanks Atsushi SAKAI Daniel Veillard <veillard@redhat.com> wrote:
On Fri, Jun 15, 2007 at 06:26:53PM -0400, Mark Johnson wrote:
Saving the dom0 patch for last :-)
On 6/15/07, Daniel Veillard <veillard@redhat.com> wrote:
On Thu, Jun 14, 2007 at 11:27:21PM +0100, Daniel P. Berrange wrote:
On Thu, Jun 14, 2007 at 05:06:16PM -0400, Mark Johnson wrote:
This is another patch which may not be popular? Xen's extra version does not fit in libvirt's release field (since it's part of an int).
Instead of printing out the wrong value, just display major.minor in virsh.
Hmm, so with Xen we have two backend impls of the Version API, one talking to the hypervisor which only ever returns the first 2 components, and the other talking to XenD which processes all 3.
As you say, in practice the extra version from Xen is effectively garbage
So while as root I see
# virsh version Compiled against library: libvir 0.2.2 Using library: libvir 0.2.2 Using API: Xen 3.0.1 Running hypervisor: Xen 3.1.0
If run as non-root I instead seee
$ virsh version Compiled against library: libvir 0.2.2 Using library: libvir 0.2.2 Using API: Xen 3.0.1 Running hypervisor: Xen 3.730.259
I think instead of this patch to change the virsh driver though, we should change teh xend_internal.c file to ignore the extra_version data from XenD as there's no way to meaningfully interpret it as an int.
Agreed, let's fix at the source instead of just dropping the data in the user land tool. I'm more concerned by getting the API right.
I'm not sure where to go from here..
Is the suggestion to change xend_internal/xen_internal to not set rev and remove the display of the hypervisor rev in virsh? Or is it something else?
Well ideally I would prefer xend to not send something crazy for 'release' but since that's unlikely, and since the hypervisor only provide major/minor anyway, let's just drop the release in src/xend_internal.c , I'm suggesting the enclosed patch,
Daniel
-- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/

On Mon, Jun 18, 2007 at 06:36:22PM +0900, Atsushi SAKAI wrote:
Hi, Daniel
I would prefer keep extra in code. (extra region stores additional info)
for example (in extra) xen-unstable : -unstable fedora7 :.4-1-1.2898.2.3
Not possible as the API allows only for an unsigned long int virConnectGetVersion (virConnectPtr conn, unsigned long *hvVer); What you want would require adding a new API, this can't be retroffied in the current one. The patch fix the current one. I'm not sure it's worth adding a new API to get more details at this point. Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/

Hi, Daniel OK, I understood. So extra description is future work. (but priority is low.) Thanks Atsushi SAKAI Daniel Veillard <veillard@redhat.com> wrote:
On Mon, Jun 18, 2007 at 06:36:22PM +0900, Atsushi SAKAI wrote:
Hi, Daniel
I would prefer keep extra in code. (extra region stores additional info)
for example (in extra) xen-unstable : -unstable fedora7 :.4-1-1.2898.2.3
Not possible as the API allows only for an unsigned long
int virConnectGetVersion (virConnectPtr conn, unsigned long *hvVer);
What you want would require adding a new API, this can't be retroffied in the current one. The patch fix the current one. I'm not sure it's worth adding a new API to get more details at this point.
Daniel
-- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/

On Mon, Jun 18, 2007 at 06:36:22PM +0900, Atsushi SAKAI wrote:
Hi,?$B!!Daniel
?$B!!I would prefer keep extra in code. (extra region stores additional info)
for example (in extra) xen-unstable : -unstable fedora7 :.4-1-1.2898.2.3
The extra release info is essentially complete garbage. Everyone who builds Xen seems to stick different random garbage in it. So its not really any use as a version number even if we wanted it :-( Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|

On 6/15/07, Daniel Veillard <veillard@redhat.com> wrote: Is the suggestion to change xend_internal/xen_internal to not set rev and remove the display of the hypervisor rev in virsh? Or is it something else?
Well ideally I would prefer xend to not send something crazy for 'release' but since that's unlikely, and since the hypervisor only provide major/minor anyway, let's just drop the release in src/xend_internal.c , I'm suggesting the enclosed patch,
Ah, I see you applied the xend patch... Not sure if you wanted/didn't want the virsh version patch too? In case you did, I've attached a slightly updated patch to not print the rel number in virsh version. Thanks, Mark

On Tue, Jun 19, 2007 at 10:31:02AM -0400, Mark Johnson wrote:
On 6/15/07, Daniel Veillard <veillard@redhat.com> wrote: Is the suggestion to change xend_internal/xen_internal to not set rev and remove the display of the hypervisor rev in virsh? Or is it something else?
Well ideally I would prefer xend to not send something crazy for 'release' but since that's unlikely, and since the hypervisor only provide major/minor anyway, let's just drop the release in src/xend_internal.c , I'm suggesting the enclosed patch,
Ah, I see you applied the xend patch... Not sure if you wanted/didn't want the virsh version patch too?
In case you did, I've attached a slightly updated patch to not print the rel number in virsh version.
No, want to keep the 3 digit numbers in virsh. Even though Xen can only give us 2 meaningful digits, the QEMU driver does have 3. Regards, Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|

On 6/19/07, Daniel P. Berrange <berrange@redhat.com> wrote:
On Tue, Jun 19, 2007 at 10:31:02AM -0400, Mark Johnson wrote:
On 6/15/07, Daniel Veillard <veillard@redhat.com> wrote:
No, want to keep the 3 digit numbers in virsh. Even though Xen can only give us 2 meaningful digits, the QEMU driver does have 3.
OK, thanks. Mark.
participants (4)
-
Atsushi SAKAI
-
Daniel P. Berrange
-
Daniel Veillard
-
Mark Johnson