Re: [Libvir] PATCH: Implement CDROM media change for QEMU/KVM driver

2007-10-15 Thread Richard W.M. Jones

+char *cmd;
+char *reply;
+/* XXX QEMU only supports a single CDROM for now */
+/*cmd = malloc(strlen(change ) + strlen(olddisk-dst) + 1 + 
strlen(newdisk-src) + 2);*/
+cmd = malloc(strlen(change ) + strlen(cdrom) + 1 + 
strlen(newdisk-src) + 2);

+if (!cmd) {
+qemudReportError(dom-conn, dom, NULL, VIR_ERR_NO_MEMORY, 
monitor command);

+return -1;
+}
+strcpy(cmd, change );
+/* XXX QEMU only supports a single CDROM for now */
+/*strcat(cmd, olddisk-dst);*/
+strcat(cmd, cdrom);
+strcat(cmd,  );
+strcat(cmd, newdisk-src);
+strcat(cmd, \n);

Much as it irritates me to say it, a fixed-size buffer and snprintf 
might be preferable here ...


Rich.

--
Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/
Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod
Street, Windsor, Berkshire, SL4 1TE, United Kingdom.  Registered in
England and Wales under Company Registration No. 03798903


smime.p7s
Description: S/MIME Cryptographic Signature
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [Libvir] PATCH: Fix const-ness for attach/detach device APIs

2007-10-15 Thread Richard W.M. Jones

Daniel P. Berrange wrote:

virDomainAttachDevice and virDomainDetachDevice both take a char * for the
XML description, but this is mistakenly not declared to be const. This 
patch fixes the public header files  all the internal drivers. NB, yes this

is in the public API, no it won't break any apps since these are input
params only.


+1

Rich.

--
Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/
Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod
Street, Windsor, Berkshire, SL4 1TE, United Kingdom.  Registered in
England and Wales under Company Registration No. 03798903


smime.p7s
Description: S/MIME Cryptographic Signature
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [Libvir] PATCH: Implement CDROM media change for QEMU/KVM driver

2007-10-15 Thread Richard W.M. Jones

Richard W.M. Jones wrote:

+strcat(cmd, newdisk-src);


Also, is quoting/escaping required?  In a naive libvirt-based app, it's 
plausible that the string is provided by the user and could contain \n 
to send arbitrary commands to the qemu console.


Rich.

--
Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/
Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod
Street, Windsor, Berkshire, SL4 1TE, United Kingdom.  Registered in
England and Wales under Company Registration No. 03798903


smime.p7s
Description: S/MIME Cryptographic Signature
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [Libvir] Core dump while executing virsh in RHEL5 .

2007-10-15 Thread Daniel Veillard
On Mon, Oct 15, 2007 at 01:10:45PM +0100, Richard W.M. Jones wrote:
 Did this problem get fixed while I was away?
 
 What I'm seeing in Veerendra's valgrind log are the following suspicious 
 messages, although the line numbers don't correspond to the earlier line 
 numbers from gdb:

  yeah, I looked at it but could not find the right mapping

 ==17508== Invalid read of size 8
 ==17508==at 0x4C5D0BB: doRemoteOpen (remote_internal.c:323)
 ==17508==by 0x4C5EABB: remoteNetworkOpen (remote_internal.c:2392)
 ==17508==by 0x4C395DA: do_open (libvirt.c:447)
 ==17508==by 0x40A80D: main (virsh.c:4507)
 
 if (uri-user) {
 username = strdup (uri-user);--- line 323
 if (!username) goto out_of_memory;
 }
 
 
 ==17508== Invalid write of size 8
 ==17508==at 0x4C5D455: doRemoteOpen (remote_internal.c:761)
 ==17508==by 0x4C5EABB: remoteNetworkOpen (remote_internal.c:2392)
 ==17508==by 0x4C395DA: do_open (libvirt.c:447)
 ==17508==by 0x40A80D: main (virsh.c:4507)
 
 if (query_out) *query_out = NULL; -- line 761
 
 As I understand the valgrind message, these indicate that the memory 
 being read/written is not valid (ie. outside any allocated malloc block 
 or static memory), although I don't understand how those lines could 
 generate that error.

  yesh I could not understand either, I was afraid the compilation with
optimization would lead to the skew of lines, and only a valgrind of 
a debug version possibly recompiled locally would be really okay to
check this out. And then the issue got off my radar :-\

Daniel

-- 
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard  | virtualization library  http://libvirt.org/
[EMAIL PROTECTED]  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/

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


Re: [Libvir] PATCH: Fix const-ness for attach/detach device APIs

2007-10-15 Thread Daniel Veillard
On Mon, Oct 15, 2007 at 12:49:34PM +0100, Richard W.M. Jones wrote:
 Daniel P. Berrange wrote:
 virDomainAttachDevice and virDomainDetachDevice both take a char * for the
 XML description, but this is mistakenly not declared to be const. This 
 patch fixes the public header files  all the internal drivers. NB, yes 
 this
 is in the public API, no it won't break any apps since these are input
 params only.
 
 +1

  yes this is safe +1

Daniel

-- 
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard  | virtualization library  http://libvirt.org/
[EMAIL PROTECTED]  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/

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


Re: [Libvir] Thoughts on remote storage support

2007-10-15 Thread Richard W.M. Jones

Tóth István wrote:

Richard W.M. Jones wrote:
For example, here are the domains and their images running on my Xen 
host at the moment.  I got this by writing a simple script which 
parses the domain XML:


fc6_0:
/var/lib/xen/images/fc6_0.img - xvda
/var/lib/xen/images/home.disk - xvdb
fc6_1:
/var/lib/xen/images/fc6_1.img - xvda
debian32fv:
/var/lib/xen/images/debian32fv.img - hda
f764pv:
/dev/Images/f764pv - xvda
freebsd32fv:
/var/lib/xen/images/freebsd32fv.img - hda
[CD] - hdc
gentoo32fv:
/var/lib/xen/images/gentoo32fv.img - hda
Well, this is a good handle for the images that belong too an active 
domain.
But I can see other images laying around, backup images, snapshots, 
virgin installed images for provisioning of new VMs, and you need to 
refer to those as well.
Hence, I still think that it would be better to use host+path. For 
example, you need to be able to say in effect: copy 
/var/lib/xen/images/fc6_0.img to /backups/fc6_xvda_1, and you have 
to refer to target image somehow.
You could just use the local path in this case, but I think that being 
able work with images on other libvirt hosts would be a bonus.


There's an open-ended access control problem here.  libvirtd runs as 
root and host+path gives a way to read and write any file on the system.


Better might be to allow the system administrator to configure 
directories where backup images, snapshots and so on may be located 
(through /etc/libvirtd.conf), and have libvirtd check this, and also 
have an additional level of enforcement through SELinux (as is done with 
Xen images now).


For my rather limited needs with virt-df I was going to propose an API 
like this:


  virDomainPeekDevice (virDomainPtr domain,
   const char *path,
   off_t offset, off_t size,
   char *result_buffer);

The security check would be something along these lines:

 * path must be a source device (as returned in the domain XML)
 * path must belong to the domain
 * offset, size must be entirely within the path device/file

This check could be extended to allow path to be in the configured 
backup / snapshot directories.


(This is not really thought through at the moment, however comments 
welcome).


With that call we then need to look at virtualising libparted so that 
instead of making direct read(2), lseek(2) etc. system calls, these may 
be redirected through a VFS layer which would call virDomainPeekDevice.


(I'm sure I posted something about this to the list, but that was two 
weeks ago, I've been on holiday, I'm jetlagged, and now I can't find it...)


[...]
In fact, the more I thought about it, and the more scenarios popped into 
my mind (plus the ones you describe above), the more I think that at 
least an initial implementation should not try to see into the 
partition, exactly because of the problems you mention above.
Even if partition/filesystem handling is included in libvirt, it should 
probably be somewhat orthogonal to the rest of the image handling 
functions.


i.e. the operations I detailed below (except for the growfs-related 
ones) for creating, moving, backing up, etc. of raw images, and a 
different set of operations that partitions, adds/removes paritions, 
creates file systems, growfs-es, etc.


This limits the complexity to just supporting simple files, block 
devices, and LVMs ( or the equivalent functionality on other platforms), 
and the parted-like functionality can be added on top of it.


My thinking about this moved along a bit: What if we explicitly _don't_ 
think about supporting LVM operations and so on within libvirt.  Making 
a general-purpose solution is a big, intractable problem.


Instead we could allow the system administrator to create some 
operations (again, through /etc/libvirtd.conf [1]):


- /etc/libvirtd.conf ---

allocate partition: lvcreate -L %size -n %name XenVolGroup
list partitions: lvs



On the libvirt side those turn into standard calls like:

  virConnectListPartitions (...)

If the commands don't exist in libvirtd.conf then those calls fail with 
a suitable error message.


We can set up suggested commands in the default configuration for Linux 
+ LVM, Linux + partitions, Solaris, etc. but it defers the policy to 
system administrators.


Rich.

[1] libvirtd.conf is only available in the remote case, so perhaps we 
need also a libvirt.conf for the local case.


--
Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/
Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod
Street, Windsor, Berkshire, SL4 1TE, United Kingdom.  Registered in
England and Wales under Company Registration No. 03798903



smime.p7s
Description: S/MIME Cryptographic Signature
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [Libvir] [PATCH] Topology fix for no cpus

2007-10-15 Thread Richard W.M. Jones

beth kon wrote:

I was able to test on a 128-way NUMA box and found a bug.


Nice :-)  What does the topology XML look like for such a beast?

Rich.

--
Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/
Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod
Street, Windsor, Berkshire, SL4 1TE, United Kingdom.  Registered in
England and Wales under Company Registration No. 03798903


smime.p7s
Description: S/MIME Cryptographic Signature
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [Libvir] remote access ?

2007-10-15 Thread Richard W.M. Jones

Mark Johnson wrote:

[EMAIL PROTECTED] ~]# virsh connect xen+tcp://localhost
libvir: Remote error : Connection refused
error: Failed to connect to the hypervisor


IME this usually happens because I forget to open the right port on the 
firewall (both incoming and outgoing ...)


Rich.

--
Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/
Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod
Street, Windsor, Berkshire, SL4 1TE, United Kingdom.  Registered in
England and Wales under Company Registration No. 03798903


smime.p7s
Description: S/MIME Cryptographic Signature
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[Libvir] [PATCH] [REPOST] Remove virDomainRestart typedef from the public API

2007-10-15 Thread Richard W.M. Jones
... however it is still used internally by the test driver, so the 
typedef is moved into src/test.c.


Rich.

--
Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/
Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod
Street, Windsor, Berkshire, SL4 1TE, United Kingdom.  Registered in
England and Wales under Company Registration No. 03798903
Index: include/libvirt/libvirt.h
===
RCS file: /data/cvs/libvirt/include/libvirt/libvirt.h,v
retrieving revision 1.58
diff -u -p -r1.58 libvirt.h
--- include/libvirt/libvirt.h	30 Sep 2007 21:09:29 -	1.58
+++ include/libvirt/libvirt.h	15 Oct 2007 14:25:55 -
@@ -67,18 +67,6 @@ typedef enum {
 } virDomainState;
 
 /**
- * virDomainRestart:
- *
- * Flags that determine the action to take on a shutdown or crash of a domain
- */
-typedef enum {
- VIR_DOMAIN_DESTROY	= 1, /* destroy the domain */
- VIR_DOMAIN_RESTART	= 2, /* restart the domain */
- VIR_DOMAIN_PRESERVE= 3, /* keep as is, need manual destroy, for debug */
- VIR_DOMAIN_RENAME_RESTART= 4/* restart under an new unique name */
-} virDomainRestart;
-
-/**
  * virDomainInfoPtr:
  *
  * a virDomainInfo is a structure filled by virDomainGetInfo() and extracting
Index: include/libvirt/libvirt.h.in
===
RCS file: /data/cvs/libvirt/include/libvirt/libvirt.h.in,v
retrieving revision 1.36
diff -u -p -r1.36 libvirt.h.in
--- include/libvirt/libvirt.h.in	30 Sep 2007 13:09:07 -	1.36
+++ include/libvirt/libvirt.h.in	15 Oct 2007 14:25:56 -
@@ -67,18 +67,6 @@ typedef enum {
 } virDomainState;
 
 /**
- * virDomainRestart:
- *
- * Flags that determine the action to take on a shutdown or crash of a domain
- */
-typedef enum {
- VIR_DOMAIN_DESTROY	= 1, /* destroy the domain */
- VIR_DOMAIN_RESTART	= 2, /* restart the domain */
- VIR_DOMAIN_PRESERVE= 3, /* keep as is, need manual destroy, for debug */
- VIR_DOMAIN_RENAME_RESTART= 4/* restart under an new unique name */
-} virDomainRestart;
-
-/**
  * virDomainInfoPtr:
  *
  * a virDomainInfo is a structure filled by virDomainGetInfo() and extracting
Index: src/test.c
===
RCS file: /data/cvs/libvirt/src/test.c,v
retrieving revision 1.49
diff -u -p -r1.49 test.c
--- src/test.c	30 Sep 2007 13:09:07 -	1.49
+++ src/test.c	15 Oct 2007 14:25:57 -
@@ -44,6 +44,15 @@
 #include buf.h
 #include uuid.h
 
+/* Flags that determine the action to take on a shutdown or crash of a domain
+ */
+typedef enum {
+ VIR_DOMAIN_DESTROY	= 1, /* destroy the domain */
+ VIR_DOMAIN_RESTART	= 2, /* restart the domain */
+ VIR_DOMAIN_PRESERVE= 3, /* keep as is, need manual destroy, for debug */
+ VIR_DOMAIN_RENAME_RESTART= 4/* restart under an new unique name */
+} virDomainRestart;
+
 struct _testDev {
 char name[20];
 int mode;


smime.p7s
Description: S/MIME Cryptographic Signature
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [Libvir] remote access ?

2007-10-15 Thread Daniel Veillard
On Mon, Oct 15, 2007 at 09:26:26AM -0400, Mark Johnson wrote:
 On 10/15/07, Richard W.M. Jones [EMAIL PROTECTED] wrote:
  Mark Johnson wrote:
   [EMAIL PROTECTED] ~]# virsh connect xen+tcp://localhost
   libvir: Remote error : Connection refused
   error: Failed to connect to the hypervisor
 
  IME this usually happens because I forget to open the right port on the
  firewall (both incoming and outgoing ...)
 
 Thanks Rich.  My problem was I typed the line wrong.  It needed
 a trailing /
 
  e.g.
 
  [EMAIL PROTECTED] ~]# virsh connect xen+tcp://localhost
 
 Should have been...
   [EMAIL PROTECTED] ~]# virsh connect xen+tcp://localhost/

  Hum, strange, that's still a valid absolute URI except with an empty
path I find strange we have a problem with this, as we don't use the 
path for the access in practice ...

Daniel

-- 
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard  | virtualization library  http://libvirt.org/
[EMAIL PROTECTED]  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/

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


Re: [Libvir] PATCH: Implement CDROM media change for QEMU/KVM driver

2007-10-15 Thread Jim Paris
Richard W.M. Jones wrote:
 +char *cmd;
 +char *reply;
 +/* XXX QEMU only supports a single CDROM for now */
 +/*cmd = malloc(strlen(change ) + strlen(olddisk-dst) + 1 + 
 strlen(newdisk-src) + 2);*/
 +cmd = malloc(strlen(change ) + strlen(cdrom) + 1 + 
 strlen(newdisk-src) + 2);
 +if (!cmd) {
 +qemudReportError(dom-conn, dom, NULL, VIR_ERR_NO_MEMORY, 
 monitor command);
 +return -1;
 +}
 +strcpy(cmd, change );
 +/* XXX QEMU only supports a single CDROM for now */
 +/*strcat(cmd, olddisk-dst);*/
 +strcat(cmd, cdrom);
 +strcat(cmd,  );
 +strcat(cmd, newdisk-src);
 +strcat(cmd, \n);
 
 Much as it irritates me to say it, a fixed-size buffer and snprintf 
 might be preferable here ...

Or asprintf.

-jim

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


Re: [Libvir] [PATCH] Enhanced stats for fullvirt domains

2007-10-15 Thread Richard W.M. Jones

Jim Meyering wrote:

+/* In Xenstore, /local/domain/0/backend/vbd/domid/device/state,
+ * if available, must be XenbusStateConnected (= 4), otherwise there
+ * is no connected device.
+ */
+static int
+check_bd_connected (xenUnifiedPrivatePtr priv, int device, int domid)
+{
+char s[256], *rs;
+int r;
+unsigned len = 0;
+
+/* This code assumes we're connected if we can't get to
+ * xenstore, etc.
+ */
+if (!priv-xshandle) return 1;
+snprintf (s, sizeof s, /local/domain/0/backend/vbd/%d/%d/state,
+  domid, device);
+s[sizeof s - 1] = '\0';
+
+rs = xs_read (priv-xshandle, 0, s, len);
+if (!rs) return 1;
+
+r = STREQ (rs, 4);
+free (rs);
+return r;
+}


That function should also check LEN (i.e., what if it's 0 or 1?).
Otherwise, STREQ might read uninitialized memory.


I've addressed that by doing something, hopefully the right thing, if 
len == 0.  (We would expect len == 1).



The unsigned len = 0; initialization looks unnecessary.


I left that because I can't see it doing any harm in this case.


==

+int
+xenLinuxDomainBlockStats (xenUnifiedPrivatePtr priv,
+  virDomainPtr dom,
+  const char *path,
+  struct _virDomainBlockStats *stats)
+{
+int minor, device;
+
+/* Paravirt domains:
+ * Paths have the form xvd[a-] and map to paths
+ * /sys/devices/xen-backend/(vbd|tap)-domid-major:minor/
+ * statistics/(rd|wr|oo)_req.
+ * The major:minor is in this case fixed as 202*256 + minor*16
+ * where minor is 0 for xvda, 1 for xvdb and so on.
+ */
+if (strlen (path) == 4 
+STREQLEN (path, xvd, 3)) {
+if ((minor = path[3] - 'a')  0 || minor  26) {
+statsErrorFunc (VIR_ERR_INVALID_ARG, __FUNCTION__,
+invalid path, should be xvda, xvdb, etc.,
+dom-id);
+return -1;
+}
+device = XENVBD_MAJOR * 256 + minor * 16;
+
+return read_bd_stats (priv, device, dom-id, stats);
+}
+/* Fullvirt domains:
+ * hda, hdb etc map to major = HD_MAJOR*256 + minor*16.
+ */
+else if (strlen (path) == 3 
+ STREQLEN (path, hd, 2)) {
+if ((minor = path[2] - 'a')  0 || minor  26) {
+statsErrorFunc (VIR_ERR_INVALID_ARG, __FUNCTION__,
+invalid path, should be hda, hdb, etc.,
+dom-id);
+return -1;
+}
+device = HD_MAJOR * 256 + minor * 16;


Probably won't ever happen, but it looks like the code
should require that minor letters be in [a-o] (aka ['a'..'a'+15]).
If it were to allow larger ones, the minor * 16 part
would be large enough to modify the major number bits of device.
That sounds undesirable.

So, if this is something to worry about, you could change
minor  26 to minor  15 in the two tests above.


Oh dear, that's a mess isn't it.  I can't find out how the devices above 
xvdo should be numbered, so I took your advice and limited the minor to 
= 15.  The same for devices = hdo too.



==

+static void
+statsErrorFunc(virErrorNumber error, const char *func, const char *info,
+   int value)
+{
+char fullinfo[1000];
+const char *errmsg;
+
+errmsg = __virErrorMsg(error, info);
+if (func != NULL) {
+snprintf(fullinfo, 999, %s: %s, func, info);
+fullinfo[999] = 0;
+__virRaiseError(NULL, NULL, NULL, VIR_FROM_XEN, error, VIR_ERR_ERROR,
+errmsg, fullinfo, NULL, value, 0, errmsg, fullinfo,
+value);
+} else {
+__virRaiseError(NULL, NULL, NULL, VIR_FROM_XEN, error, VIR_ERR_ERROR,
+errmsg, info, NULL, value, 0, errmsg, info,
+value);


Use sizeof fullinfo - 1, not 999.
Rather than duplicating the __virRaiseError call,
how about calling it just once, with info or fullinfo?


Yes, the perils of copying and pasting code.  Fixed both.


==

+static int
+read_bd_stats (xenUnifiedPrivatePtr priv,
...
+/* 'Bytes' was really sectors when we read it.  Scale up by
+ * an assumed sector size.
+ */
+if (stats-rd_bytes  0) stats-rd_bytes *= 512;
+if (stats-wr_bytes  0) stats-wr_bytes *= 512;


For large starting values of rd_bytes and wr_bytes,
it'd be nice if rather than wrapping around, the result
were the maximum value for that type -- or maybe a sentinel value.


It would have to be really really large (these are 64 bit numbers), but 
yes I have fixed this too.


Updated patch is attached.

Thanks,

Rich.

--
Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/
Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod
Street, Windsor, Berkshire, SL4 1TE, 

Re: [Libvir] [PATCH] Enhanced stats for fullvirt domains

2007-10-15 Thread Jim Meyering
Richard W.M. Jones [EMAIL PROTECTED] wrote:
...
 That function should also check LEN (i.e., what if it's 0 or 1?).
 Otherwise, STREQ might read uninitialized memory.

 I've addressed that by doing something, hopefully the right thing, if
 len == 0.  (We would expect len == 1).

Yeah, you're right.  len==0 was the only missed case.
I didn't realize at the time that len doesn't include
the trailing NUL byte.

 The unsigned len = 0; initialization looks unnecessary.

 I left that because I can't see it doing any harm in this case.

It's not a big deal.
True, it causes no run-time harm, but anything unnecessary
can make the reviewer/maintainer wonder why?.
...
 It would have to be really really large (these are 64 bit numbers),
 but yes I have fixed this too.

 Updated patch is attached.

Looks fine, now.

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


Re: [Libvir] remote access ?

2007-10-15 Thread Daniel P. Berrange
On Mon, Oct 15, 2007 at 04:37:22PM +0100, Richard W.M. Jones wrote:
 Daniel P. Berrange wrote:
 It is a bug in xend_internal.c
 
 if (strcasecmp (name, xen) == 0 ||
 strncasecmp (name, xen:///, 7) == 0) {
 
 
 This needs to die  be replaced with code calling the libxml URI parsing.
 I fixed a similar bug in the QEMU driver a few weeks back.
 
 Sorry yes, Dan is right, so ignore the hack/patch I just posted.
 
 In fact xen_unified.c already parses the URI once, so we just need to 
 change the xen_unified - xen low level drivers API to pass the parsed 
 URI instead of the name string.

In fact we could take it one bit further. Have the 'do_open' method in
the src/libvirt.c file parse it  change the src/driver.h so that the
open method for drivers takes  xmlURIPtr instead of a char *. That way
we guarentee that all drivers will use a correctly parsed form.


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  -=| 

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


Re: [Libvir] remote access ?

2007-10-15 Thread Daniel Veillard
On Mon, Oct 15, 2007 at 08:33:08PM +0100, Daniel P. Berrange wrote:
 On Mon, Oct 15, 2007 at 04:37:22PM +0100, Richard W.M. Jones wrote:
  Daniel P. Berrange wrote:
  It is a bug in xend_internal.c
  
  if (strcasecmp (name, xen) == 0 ||
  strncasecmp (name, xen:///, 7) == 0) {
  
  
  This needs to die  be replaced with code calling the libxml URI parsing.
  I fixed a similar bug in the QEMU driver a few weeks back.
  
  Sorry yes, Dan is right, so ignore the hack/patch I just posted.
  
  In fact xen_unified.c already parses the URI once, so we just need to 
  change the xen_unified - xen low level drivers API to pass the parsed 
  URI instead of the name string.
 
 In fact we could take it one bit further. Have the 'do_open' method in
 the src/libvirt.c file parse it  change the src/driver.h so that the
 open method for drivers takes  xmlURIPtr instead of a char *. That way
 we guarentee that all drivers will use a correctly parsed form.

  Sounds good but we would have to reserialize for the network access
I assume.

Daniel

-- 
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard  | virtualization library  http://libvirt.org/
[EMAIL PROTECTED]  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/

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