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@xxxxxxxxxx> 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@xxxxxxxxxx | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
Index: src/xend_internal.c =================================================================== RCS file: /data/cvs/libxen/src/xend_internal.c,v retrieving revision 1.119 diff -u -r1.119 xend_internal.c --- src/xend_internal.c 15 Jun 2007 15:24:20 -0000 1.119 +++ src/xend_internal.c 18 Jun 2007 08:39:38 -0000 @@ -2601,8 +2601,7 @@ xenDaemonGetVersion(virConnectPtr conn, unsigned long *hvVer) { struct sexpr *root; - const char *extra; - int major, minor, release = 0; + int major, minor; unsigned long version; if (!VIR_IS_CONNECT(conn)) { @@ -2619,16 +2618,8 @@ major = sexpr_int(root, "node/xen_major"); minor = sexpr_int(root, "node/xen_minor"); - extra = sexpr_node(root, "node/xen_extra"); - if (extra != NULL) { - while (*extra != 0) { - if ((*extra >= '0') && (*extra <= '9')) - release = release * 10 + (*extra - '0'); - extra++; - } - } sexpr_free(root); - version = major * 1000000 + minor * 1000 + release; + version = major * 1000000 + minor * 1000; *hvVer = version; return(0); }