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.

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.

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(a)redhat.com&gt; 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(a)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(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/

> On 6/15/07, Daniel Veillard <veillard(a)redhat.com&gt; 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,

On Tue, Jun 19, 2007 at 10:31:02AM -0400, Mark Johnson wrote: > >> On 6/15/07, Daniel Veillard <veillard(a)redhat.com&gt; 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.