Re: [Libvir] [PATCH] lxc: handle SIGCHLD from exiting container

[Dave Leskovec]

Jim Meyering wrote:
 Dave Leskovec [EMAIL PROTECTED] wrote:
 ...
 +#ifndef   _SIGNAL_H
 +#include signal.h
 +#endif
 In practice it's fine to include signal.h unconditionally,
 and even multiple times.  Have you encountered a version of signal.h
 that may not be included twice?  If so, it probably deserves a comment
 with the details.
 No, I don't have any special condition here.  This is probably some past
 conditioning resurfacing briefly.  If I remember correctly, it had more to do
 with compile efficiency rather than avoiding compile failures from multiple
 inclusions.
 
 Then don't bother.
 gcc performs a handy optimization whereby it doesn't even open
 the header file the second (and subsequent) time it's included, as
 long as it's entire contents is wrapped in the usual sort of guard:
 
   #ifndef SYM
   #define SYM
   ...
   #endif

Thanks Jim.  I've attached an updated patch with those two changes.  While
making these changes, I noticed that I missed updating the storage drivers state
driver table.  I've fixed that as well.

-- 
Best Regards,
Dave Leskovec
IBM Linux Technology Center
Open Virtualization
---
 qemud/qemud.c   |   26 ++
 src/driver.h|4 +
 src/internal.h  |2 
 src/libvirt.c   |   11 
 src/libvirt_sym.version |1 
 src/lxc_driver.c|  114 +++-
 src/qemu_driver.c   |1 
 src/remote_internal.c   |1 
 src/storage_driver.c|1 
 9 files changed, 120 insertions(+), 41 deletions(-)

Index: b/qemud/qemud.c
===
--- a/qemud/qemud.c	2008-04-28 02:09:52.0 -0700
+++ b/qemud/qemud.c	2008-04-30 15:34:29.0 -0700
@@ -109,16 +109,16 @@
 static sig_atomic_t sig_errors = 0;
 static int sig_lasterrno = 0;
 
-static void sig_handler(int sig) {
-unsigned char sigc = sig;
+static void sig_handler(int sig, siginfo_t * siginfo,
+void* context ATTRIBUTE_UNUSED) {
 int origerrno;
 int r;
 
-if (sig == SIGCHLD) /* We explicitly waitpid the child later */
-return;
+/* set the sig num in the struct */
+siginfo-si_signo = sig;
 
 origerrno = errno;
-r = safewrite(sigwrite, sigc, 1);
+r = safewrite(sigwrite, siginfo, sizeof(*siginfo));
 if (r == -1) {
 sig_errors++;
 sig_lasterrno = errno;
@@ -232,10 +232,10 @@
  int events ATTRIBUTE_UNUSED,
  void *opaque) {
 struct qemud_server *server = (struct qemud_server *)opaque;
-unsigned char sigc;
+siginfo_t siginfo;
 int ret;
 
-if (read(server-sigread, sigc, 1) != 1) {
+if (read(server-sigread, siginfo, sizeof(siginfo)) != sizeof(siginfo)) {
 qemudLog(QEMUD_ERR, _(Failed to read from signal pipe: %s),
  strerror(errno));
 return;
@@ -243,7 +243,7 @@
 
 ret = 0;
 
-switch (sigc) {
+switch (siginfo.si_signo) {
 case SIGHUP:
 qemudLog(QEMUD_INFO, %s, _(Reloading configuration on SIGHUP));
 if (virStateReload()  0)
@@ -253,11 +253,15 @@
 case SIGINT:
 case SIGQUIT:
 case SIGTERM:
-qemudLog(QEMUD_WARN, _(Shutting down on signal %d), sigc);
+qemudLog(QEMUD_WARN, _(Shutting down on signal %d),
+ siginfo.si_signo);
 server-shutdown = 1;
 break;
 
 default:
+qemudLog(QEMUD_INFO, _(Received signal %d, dispatching to drivers),
+ siginfo.si_signo);
+virStateSigDispatcher(siginfo);
 break;
 }
 
@@ -2186,8 +2190,8 @@
 goto error1;
 }
 
-sig_action.sa_handler = sig_handler;
-sig_action.sa_flags = 0;
+sig_action.sa_sigaction = sig_handler;
+sig_action.sa_flags = SA_SIGINFO;
 sigemptyset(sig_action.sa_mask);
 
 sigaction(SIGHUP, sig_action, NULL);
Index: b/src/driver.h
===
--- a/src/driver.h	2008-04-10 09:54:54.0 -0700
+++ b/src/driver.h	2008-05-05 23:28:40.0 -0700
@@ -11,6 +11,8 @@
 
 #include libxml/uri.h
 
+#include signal.h
+
 #ifdef __cplusplus
 extern C {
 #endif
@@ -565,6 +567,7 @@
 typedef int (*virDrvStateCleanup) (void);
 typedef int (*virDrvStateReload) (void);
 typedef int (*virDrvStateActive) (void);
+typedef int (*virDrvSigHandler) (siginfo_t *siginfo);
 
 typedef struct _virStateDriver virStateDriver;
 typedef virStateDriver *virStateDriverPtr;
@@ -574,6 +577,7 @@
 virDrvStateCleanup cleanup;
 virDrvStateReload  reload;
 virDrvStateActive  active;
+virDrvSigHandler   sigHandler;
 };
 
 /*
Index: b/src/internal.h
===
--- a/src/internal.h	2008-04-28 14:44:54.0 -0700
+++ b/src/internal.h	2008-04-30 15:01:24.0 -0700
@@ -344,10 +344,12 @@
 int __virStateCleanup(void);
 int __virStateReload(void);
 

[Libvir] [PATCH] lxc: loop in tty forwarding process

[Dave Leskovec]

This patch changes the lxc tty forwarding process to use epoll instead of poll.
 This is done to avoid a cpu consuming loop when a user disconnects from the
container console.

During some testing, we found that when the slave end of a tty is closed, calls
to poll() on the master end will return immediately with POLLHUP until the slave
end is opened again.  The result of this is that if you connect to the container
console using virsh and then ctrl-] out of it, the container tty process will
chew up the processor handling POLLHUP.  This event can't be disabled regardless
of what is set in the events field.

This can be avoided by using epoll.  When used in edge triggered mode, you see
the initial EPOLLHUP but will not see another one until someone connects and
then disconnects from the console again.  This also drove some changes into how
the regular input data is handled.  Once an EPOLLIN is seen from an fd, another
one will not be surfaced until all data has been read from the file (EAGAIN is
returned by read).

-- 
Best Regards,
Dave Leskovec
IBM Linux Technology Center
Open Virtualization
---
 src/lxc_driver.c |  174 ---
 1 file changed, 129 insertions(+), 45 deletions(-)

Index: b/src/lxc_driver.c
===
--- a/src/lxc_driver.c	2008-05-05 23:21:56.0 -0700
+++ b/src/lxc_driver.c	2008-05-06 00:47:31.0 -0700
@@ -26,9 +26,10 @@
 #ifdef WITH_LXC
 
 #include fcntl.h
-#include poll.h
+#include sys/epoll.h
 #include sched.h
 #include sys/utsname.h
+#include stdbool.h
 #include string.h
 #include sys/types.h
 #include termios.h
@@ -552,7 +553,7 @@
 int rc = -1;
 char tempTtyName[PATH_MAX];
 
-*ttymaster = posix_openpt(O_RDWR|O_NOCTTY);
+*ttymaster = posix_openpt(O_RDWR|O_NOCTTY|O_NONBLOCK);
 if (*ttymaster  0) {
 lxcError(conn, NULL, VIR_ERR_INTERNAL_ERROR,
  _(posix_openpt failed: %s), strerror(errno));
@@ -593,75 +594,155 @@
 }
 
 /**
+ * lxcFdForward:
+ * @readFd: file descriptor to read
+ * @writeFd: file desriptor to write
+ *
+ * Reads 1 byte of data from readFd and writes to writeFd.
+ *
+ * Returns 0 on success, EAGAIN if returned on read, or -1 in case of error
+ */
+static int lxcFdForward(int readFd, int writeFd)
+{
+int rc = -1;
+char buf[2];
+
+if (1 != (saferead(readFd, buf, 1))) {
+if (EAGAIN == errno) {
+rc = EAGAIN;
+goto cleanup;
+}
+
+lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ _(read of fd %d failed: %s), readFd, strerror(errno));
+goto cleanup;
+}
+
+if (1 != (safewrite(writeFd, buf, 1))) {
+lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ _(write to fd %d failed: %s), writeFd, strerror(errno));
+goto cleanup;
+}
+
+rc = 0;
+
+cleanup:
+return rc;
+}
+
+typedef struct _lxcTtyForwardFd_t {
+int fd;
+bool active;
+} lxcTtyForwardFd_t;
+
+/**
  * lxcTtyForward:
  * @fd1: Open fd
  * @fd1: Open fd
  *
  * Forwards traffic between fds.  Data read from fd1 will be written to fd2
- * Data read from fd2 will be written to fd1.  This process loops forever.
+ * This process loops forever.
+ * This uses epoll in edge triggered mode to avoid a hard loop on POLLHUP
+ * events when the user disconnects the virsh console via ctrl-]
  *
  * Returns 0 on success or -1 in case of error
  */
 static int lxcTtyForward(int fd1, int fd2)
 {
 int rc = -1;
-int i;
-char buf[2];
-struct pollfd fds[2];
-int numFds = 0;
-
-if (0 = fd1) {
-fds[numFds].fd = fd1;
-fds[numFds].events = POLLIN;
-++numFds;
+int epollFd;
+struct epoll_event epollEvent;
+int numEvents;
+int numActive = 0;
+lxcTtyForwardFd_t fdArray[2];
+int timeout = -1;
+int curFdOff = 0;
+int writeFdOff = 0;
+
+fdArray[0].fd = fd1;
+fdArray[0].active = false;
+fdArray[1].fd = fd2;
+fdArray[1].active = false;
+
+/* create the epoll fild descriptor */
+epollFd = epoll_create(2);
+if (0  epollFd) {
+lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ _(epoll_create(2) failed: %s), strerror(errno));
+goto cleanup;
 }
 
-if (0 = fd2) {
-fds[numFds].fd = fd2;
-fds[numFds].events = POLLIN;
-++numFds;
+/* add the file descriptors the epoll fd */
+memset(epollEvent, 0x00, sizeof(epollEvent));
+epollEvent.events = EPOLLIN|EPOLLET;/* edge triggered */
+epollEvent.data.fd = fd1;
+epollEvent.data.u32 = 0;/* fdArray position */
+if (0  epoll_ctl(epollFd, EPOLL_CTL_ADD, fd1, epollEvent)) {
+lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ _(epoll_ctl(fd1) failed: %s), strerror(errno));
+goto cleanup;
 }
-
-if (0 == numFds) {
-DEBUG0(No fds to monitor, return);
+epollEvent.data.fd = fd2;
+

[Libvir] ISCSI howto, example, etc?

[Mark Dehus]

The documentation on the new storage feature is either a little short,
or I totally missed it.  Anyway, I am looking for some help on how to
use it.  If anyone could maybe give me an example xml file that would
be incredibly helpful, because based on the format description I can't
exactly figure out what information I am supposed to put where (for
ISCSI).

Thanks!

-- 
$ ./Mark

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

Re: [Libvir] RFC 'xen like scripts'

[Daniel P. Berrange]

On Tue, May 06, 2008 at 07:53:29AM +0200, Stefan de Konink wrote:
 On Tue, 6 May 2008, Daniel P. Berrange wrote:
 
   Due to the reason all LUNs are exported over one connection, a
   rescan before usage a rescan is always required. LUN numbering is not
   stable, nor they can be found at the client side.
 
  So where does the information mapping netapp pathnames to the the LUNs
  come from ?
 
 The information service, so in my case ssh script.

Having the daemon SSH into the filer is a non-starter. There's no way most
admins will allow that as in any reasonably large company its fundamental
security protocol that there is separation between server  filer admins.
One group not having access to the other's systems  vica-verca.

And from the other via SELinux policy for libvirt will not allow the 
daamon to SSH to servers as it violates the principle of containment.

  If LUNs can change when re-scanning what happens to LUNs
  already in use for other guests ? It doesn't sound usable if LUNs that
  are in use get renumbered at rescan.
 
 Luns that don't change get not remapped. But if a user decides to destroy
 a disk, and have one with the same name again, it is most likely to get a
 other lun. But with the same name.

So the LUN is stable for the entire lifetime of the associated named disk
volume.

  AFAICT this is basically just suggesting an alternate naming scheme for
  storage volumes, instead of 'lun-XXX' where XXX is the number, you want
  a name that's independant of LUN numbering. So the key question is where
  does the information for the persistent names come from  ?
 
 Exactly this is what I want. If I have a iSCSI uri, I want to have it
 discovered, if I have a netapp uri I want to have it 'discovered' too, but
 in this case I provide the address of the administrative interface.

Logging into the admin interface is a non-starter.

 And unlike you do for storage pools with iSCSI that the provided target
 name for volumes should 'match up' with the 'discovered name', I want this
 to be transparent to the user. *Because* Linux might have a target
 /dev/bla-by-path/X but who says *BSD, *Solaris has it? (Yes I know, there
 are other problems, but the base problem is that the target provided target 
 device
 is pretty limited to one OS running udev.)

The question of disk  path naming is completely independnat of of the 
storage volume namin - they are separate pieces of metadata  there's
no hard link between them. The /dev/ naming scheme for volumes is by 
neccessity going to be different across every OS. The use of /dev/disk/byXXX
is obviously Linux speciifc and would need to be adapted for any BSD
or Solaris disto.

There are several levels of naming for storage volumes in the libvirt API

  - name  - unique with in the scope of a storage pool
  - key   - unique amongst all storage pools
  - path  - unique amongst all pools in a single host

The scheme for each can be addressed independantly as best suits the storage
type in question.

Dan.
-- 
|: Red Hat, Engineering, Boston   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  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] RFC 'xen like scripts'

[Stefan de Konink]

On Tue, 6 May 2008, Stefan de Konink wrote:

 The LUN probably is, as stable as the human readable path to it. If your
 suggestion is:

 /dev/disk/by-path/ip-172.16.103.200:3260-iscsi-iqn.1992-08.com.netapp:sn.118046347:vf.88fa4694-0ba6-11dd-b8a9-00a09807592f-lun-1034

Lets elaborate a bit further on this. If this would be implemented using
libvirt as 'normal' path. No iSCSI fuss, since the host should be logged
in anyway.

How would I 'force' a rescan using libvirt as only mean of communication
with my dom0?


Stefan

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

Re: [Libvir] RFC 'xen like scripts'

[Daniel P. Berrange]

On Tue, May 06, 2008 at 02:52:40PM +0200, Stefan de Konink wrote:
 On Tue, 6 May 2008, Stefan de Konink wrote:
 
  The LUN probably is, as stable as the human readable path to it. If your
  suggestion is:
 
  /dev/disk/by-path/ip-172.16.103.200:3260-iscsi-iqn.1992-08.com.netapp:sn.118046347:vf.88fa4694-0ba6-11dd-b8a9-00a09807592f-lun-1034
 
 Lets elaborate a bit further on this. If this would be implemented using
 libvirt as 'normal' path. No iSCSI fuss, since the host should be logged
 in anyway.
 
 How would I 'force' a rescan using libvirt as only mean of communication
 with my dom0?

By invoking the  virStoragePoolRefresh method

Dan.
-- 
|: Red Hat, Engineering, Boston   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  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] RFC 'xen like scripts'

[Daniel P. Berrange]

On Tue, May 06, 2008 at 02:44:49PM +0200, Stefan de Konink wrote:
 On Tue, 6 May 2008, Daniel P. Berrange wrote:
  On Tue, May 06, 2008 at 07:53:29AM +0200, Stefan de Konink wrote:
 
  Having the daemon SSH into the filer is a non-starter. There's no way most
  admins will allow that as in any reasonably large company its fundamental
  security protocol that there is separation between server  filer admins.
  One group not having access to the other's systems  vica-verca.
 
 That is why the Net-patent me-App people invented vfilers. Next to this
 Dom0 != DomU so where is the security breach? No 'client/user' is able to
 do anything with libvirt or 'xm' so I'm very interested in what seperation
 you are talking about.

Well the use of vfilers isn't something you mentioned before, so it could
make it more viable. I'm still not particularly keen on the idea of SSHing
into machines from the libvirt daemon though. What authentication mechanims
does it use ? Passwords, kerberos GSSAPI, public keys ?

   Luns that don't change get not remapped. But if a user decides to destroy
   a disk, and have one with the same name again, it is most likely to get a
   other lun. But with the same name.
 
  So the LUN is stable for the entire lifetime of the associated named disk
  volume.
 
 The LUN probably is, as stable as the human readable path to it. If your
 suggestion is:
 
 /dev/disk/by-path/ip-172.16.103.200:3260-iscsi-iqn.1992-08.com.netapp:sn.118046347:vf.88fa4694-0ba6-11dd-b8a9-00a09807592f-lun-1034
 
 Instead of some more 'simple' and human readable way, that is *garanteed*
 to give the right paths at start, no matter what happens (for example
 maintance)... it is your argument against mine.

As I mentioned before there are several names available for a volume - the
path, the 'name', and the 'key'. The 'name' is the only one intended to 
ever be shown to the user in normal circumstances, and in your example above
that would simply be  'lun-1034'. The full stable path you illustrate above
is intended for use by management tools, not to be shown to the user.

 src/storage_backend_iscsi.c:293
 /* Now figure out the stable path
  *
  * XXX this method is O(N) because it scans the pool target
  * dir every time its run. Should figure out a more efficient
  * way of doing this...
  */
 if ((vol-target.path = virStorageBackendStablePath(conn,
 pool,
 devpath)) == NULL)
 goto cleanup;
 
 if (devpath != vol-target.path)
 free(devpath);
 devpath = NULL;
 
 What are you trying to 'check' here?

It is checking whether the input path (eg '/dev/sdf') is the same as the
returned path. If not, then it free's the original inpujt path since it
is now unused.

Dan
-- 
|: Red Hat, Engineering, Boston   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  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] ISCSI howto, example, etc?

[Daniel P. Berrange]

On Tue, May 06, 2008 at 12:50:25AM -0600, Mark Dehus wrote:
 The documentation on the new storage feature is either a little short,
 or I totally missed it.  Anyway, I am looking for some help on how to
 use it.  If anyone could maybe give me an example xml file that would
 be incredibly helpful, because based on the format description I can't
 exactly figure out what information I am supposed to put where (for
 ISCSI).

The main docs we have are

http://libvirt.org/formatstorage.html
http://libvirt.org/storage.html

They are basically more reference guides that howto/tutorials. Also
look at the 'virsh help' output for 'vol-XXX' and 'pool-XXX' commands.


As a simple example...

  - Create an XML definition for the iSCSI target you want to access,

  # cat  virt.xml EOF
  pool type=iscsi
namevirtimages/name
source
  host name=iscsi.example.com/
  device path=demo-target/
/source
target
  path/dev/disk/by-path/path
/target
  /pool
  EOF


  - Define the pool

 # virsh pool-define virt.xml

  - Activate the pool (this step logs into the server)

 # virsh pool-start virtimages

  - List the LUNs available

 # virsh vol-list virtimages

  - Query info about a LUN

 # virsh vol-info --pool virtimages  lun-3
 # virsh vol-dumpxml --pool virtimages  lun-3

There's many more pool-XXX and vol-XX commands available

Regards,
Dan.
-- 
|: Red Hat, Engineering, Boston   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  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] RFC 'xen like scripts'

[Stefan de Konink]

On Tue, 6 May 2008, Daniel P. Berrange wrote:

 On Tue, May 06, 2008 at 02:44:49PM +0200, Stefan de Konink wrote:
  On Tue, 6 May 2008, Daniel P. Berrange wrote:
   On Tue, May 06, 2008 at 07:53:29AM +0200, Stefan de Konink wrote:
  
   Having the daemon SSH into the filer is a non-starter. There's no way most
   admins will allow that as in any reasonably large company its fundamental
   security protocol that there is separation between server  filer admins.
   One group not having access to the other's systems  vica-verca.
 
  That is why the Net-patent me-App people invented vfilers. Next to this
  Dom0 != DomU so where is the security breach? No 'client/user' is able to
  do anything with libvirt or 'xm' so I'm very interested in what seperation
  you are talking about.

 Well the use of vfilers isn't something you mentioned before, so it could
 make it more viable. I'm still not particularly keen on the idea of SSHing
 into machines from the libvirt daemon though. What authentication mechanims
 does it use ? Passwords, kerberos GSSAPI, public keys ?

Lets say, NetApp 'claims' it can do it with keys. But since noone is able
to add my key for a vfiler I have now implemented as a python script using
pexcept to be able to login using ssh.

Trust me, if I was happy with the solution I would be alot more positive
about the complete implementation.


Luns that don't change get not remapped. But if a user decides to 
destroy
a disk, and have one with the same name again, it is most likely to get 
a
other lun. But with the same name.
  
   So the LUN is stable for the entire lifetime of the associated named disk
   volume.
 
  The LUN probably is, as stable as the human readable path to it. If your
  suggestion is:
 
  /dev/disk/by-path/ip-172.16.103.200:3260-iscsi-iqn.1992-08.com.netapp:sn.118046347:vf.88fa4694-0ba6-11dd-b8a9-00a09807592f-lun-1034
 
  Instead of some more 'simple' and human readable way, that is *garanteed*
  to give the right paths at start, no matter what happens (for example
  maintance)... it is your argument against mine.

 As I mentioned before there are several names available for a volume - the
 path, the 'name', and the 'key'. The 'name' is the only one intended to
 ever be shown to the user in normal circumstances, and in your example above
 that would simply be  'lun-1034'. The full stable path you illustrate above
 is intended for use by management tools, not to be shown to the user.

I'll give it a shot then :)

  src/storage_backend_iscsi.c:293
  /* Now figure out the stable path
   *
   * XXX this method is O(N) because it scans the pool target
   * dir every time its run. Should figure out a more efficient
   * way of doing this...
   */
  if ((vol-target.path = virStorageBackendStablePath(conn,
  pool,
  devpath)) == NULL)
  goto cleanup;
 
  if (devpath != vol-target.path)
  free(devpath);
  devpath = NULL;
 
  What are you trying to 'check' here?

 It is checking whether the input path (eg '/dev/sdf') is the same as the
 returned path. If not, then it free's the original inpujt path since it
 is now unused.

But is this input path the target path that is in the examples online (as
last xmlnode)?


Stefan

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

Re: [Libvir] RFC 'xen like scripts'

[Daniel P. Berrange]

On Tue, May 06, 2008 at 03:37:04PM +0200, Stefan de Konink wrote:
 On Tue, 6 May 2008, Daniel P. Berrange wrote:
   /* Now figure out the stable path
*
* XXX this method is O(N) because it scans the pool target
* dir every time its run. Should figure out a more efficient
* way of doing this...
*/
   if ((vol-target.path = virStorageBackendStablePath(conn,
   pool,
   devpath)) == NULL)
   goto cleanup;
  
   if (devpath != vol-target.path)
   free(devpath);
   devpath = NULL;
  
   What are you trying to 'check' here?
 
  It is checking whether the input path (eg '/dev/sdf') is the same as the
  returned path. If not, then it free's the original inpujt path since it
  is now unused.
 
 But is this input path the target path that is in the examples online (as
 last xmlnode)?

The 'devpath' parameter to virStorageBackendStablePath() is the path of the
LUN in question, typically eg, /dev/sdg.  The 'pool' object has an internal
attribute which is the 'target' element from the pool XML, eg 
'/dev/disk/by-path'.

The virStorageBackendStablePath(), then iterates over all the entries in
'/dev/disk/by-path' to find the one matching (symlinking to) '/dev/sdg'
and returns it, eg 
/dev/disk/by-path/ip-172.16.103.200:3260-iscsi-iqn.1992-08.com.netapp:sn.118046347:vf.88fa4694-0ba6-11dd-b8a9-00a09807592f-lun-1034
but if it doesn't find a matching symlink then it just returns the original
string '/dev/sdg'.

The latter scenario is depressingly common because soo many distros (including
RHEL-5) have broken udev rules for iSCSI. Its only since Fedora 8 that they
work properly for me

Dan
-- 
|: Red Hat, Engineering, Boston   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  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] [PATCH] lxc: handle SIGCHLD from exiting container

[Jim Meyering]

Dave Leskovec [EMAIL PROTECTED] wrote:
 Jim Meyering wrote:
 Dave Leskovec [EMAIL PROTECTED] wrote:
 ...
 +#ifndef  _SIGNAL_H
 +#include signal.h
 +#endif
 In practice it's fine to include signal.h unconditionally,
 and even multiple times.  Have you encountered a version of signal.h
 that may not be included twice?  If so, it probably deserves a comment
 with the details.
 No, I don't have any special condition here.  This is probably some past
 conditioning resurfacing briefly.  If I remember correctly, it had more to 
 do
 with compile efficiency rather than avoiding compile failures from multiple
 inclusions.

 Then don't bother.
 gcc performs a handy optimization whereby it doesn't even open
 the header file the second (and subsequent) time it's included, as
 long as it's entire contents is wrapped in the usual sort of guard:

   #ifndef SYM
   #define SYM
   ...
   #endif

 Thanks Jim.  I've attached an updated patch with those two changes.  While
 making these changes, I noticed that I missed updating the storage drivers 
 state
 driver table.  I've fixed that as well.

 --
...
 Index: b/qemud/qemud.c
 ===
...
 +static void sig_handler(int sig, siginfo_t * siginfo,
 +void* context ATTRIBUTE_UNUSED) {
...
 -unsigned char sigc;
 +siginfo_t siginfo;
  int ret;

 -if (read(server-sigread, sigc, 1) != 1) {
 +if (read(server-sigread, siginfo, sizeof(siginfo)) != sizeof(siginfo)) 
 {

Looks good, but that should be saferead instead of read.
Now that it's reading more than one byte, EINTR can make a difference.

Also, it'd make it a tiny bit easier for people who reply to you
if you did not put code after your signature.
Or at least not after the --  signature-introducer.
Some mail clients (at least Gnus) -quote only the part
of your message that comes before the signature.

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

Re: [Libvir] [PATCH] Add bus attribute to disk target definition

[Soren Hansen]

Hi,

sorry for the delay. Here's the newest version of the patch which should
address all the issues that have been raised so far.


=== modified file 'src/qemu_conf.c'
--- old/src/qemu_conf.c 2008-04-30 12:30:55 +
+++ new/src/qemu_conf.c 2008-05-06 17:58:44 +
@@ -491,6 +491,8 @@
 *flags |= QEMUD_CMD_FLAG_KQEMU;
 if (strstr(help, -no-reboot))
 *flags |= QEMUD_CMD_FLAG_NO_REBOOT;
+if (strstr(help, \n-drive))
+*flags |= QEMUD_CMD_FLAG_DRIVE_OPT;
 if (*version = 9000)
 *flags |= QEMUD_CMD_FLAG_VNC_COLON;
 ret = 0;
@@ -568,6 +570,7 @@
 xmlChar *source = NULL;
 xmlChar *target = NULL;
 xmlChar *type = NULL;
+xmlChar *bus = NULL;
 int typ = 0;
 
 type = xmlGetProp(node, BAD_CAST type);
@@ -598,6 +601,7 @@
 } else if ((target == NULL) 
(xmlStrEqual(cur-name, BAD_CAST target))) {
 target = xmlGetProp(cur, BAD_CAST dev);
+bus = xmlGetProp(cur, BAD_CAST bus);
 } else if (xmlStrEqual(cur-name, BAD_CAST readonly)) {
 disk-readonly = 1;
 }
@@ -643,10 +647,9 @@
 disk-readonly = 1;
 
 if ((!device || !strcmp((const char *)device, disk)) 
-strcmp((const char *)target, hda) 
-strcmp((const char *)target, hdb) 
-strcmp((const char *)target, hdc) 
-strcmp((const char *)target, hdd)) {
+strncmp((const char *)target, hd, 2) 
+strncmp((const char *)target, sd, 2) 
+strncmp((const char *)target, vd, 2)) {
 qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
  _(Invalid harddisk device name: %s), target);
 goto error;
@@ -673,13 +676,28 @@
 goto error;
 }
 
+if (!bus)
+disk-bus = QEMUD_DISK_BUS_IDE;
+else if (STREQ((const char *)bus, ide))
+disk-bus = QEMUD_DISK_BUS_IDE;
+else if (STREQ((const char *)bus, scsi))
+disk-bus = QEMUD_DISK_BUS_SCSI;
+else if (STREQ((const char *)bus, virtio))
+disk-bus = QEMUD_DISK_BUS_VIRTIO;
+else {
+qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _(Invalid 
bus type: %s), bus);
+goto error;
+}
+
 xmlFree(device);
 xmlFree(target);
 xmlFree(source);
+xmlFree(bus);
 
 return 0;
 
  error:
+xmlFree(bus);
 xmlFree(type);
 xmlFree(target);
 xmlFree(source);
@@ -1373,6 +1391,68 @@
 return -1;
 }
 
+static int qemudDiskCompare(const void *aptr, const void *bptr) {
+struct qemud_vm_disk_def *a = (struct qemud_vm_disk_def *) aptr;
+struct qemud_vm_disk_def *b = (struct qemud_vm_disk_def *) bptr;
+if (a-device == b-device)
+return virDiskNameToIndex(a-dst) - virDiskNameToIndex(b-dst);
+else
+return a-device - b-device;
+}
+
+static const char *qemudBusIdToName(int busId) {
+const char *busnames[] = { ide,
+scsi,
+virtio };
+
+   if (busId = 0  busId  3)
+   return busnames[busId];
+   else
+   return 0;
+}
+
+static char *qemudDriveOpt(struct qemud_vm_disk_def *disk, int boot)
+{
+char opt[PATH_MAX];
+
+switch (disk-device) {
+case QEMUD_DISK_CDROM:
+snprintf(opt, PATH_MAX, file=%s,if=ide,media=cdrom%s,
+  disk-src, boot ? ,boot=on : );
+break;
+case QEMUD_DISK_FLOPPY:
+snprintf(opt, PATH_MAX, file=%s,if=floppy%s,
+  disk-src, boot ? ,boot=on : );
+break;
+case QEMUD_DISK_DISK:
+snprintf(opt, PATH_MAX, file=%s,if=%s%s,
+  disk-src, qemudBusIdToName(disk-bus), boot ? 
,boot=on : );
+break;
+default:
+return 0;
+}
+return strdup(opt);
+}
+
+static char *qemudAddBootDrive(virConnectPtr conn,
+struct qemud_vm_def *def,
+   char *handledDisks,
+   int type) {
+int j = 0;
+struct qemud_vm_disk_def *disk = def-disks;
+
+while (disk) {
+if (!handledDisks[j]  disk-device == type) {
+handledDisks[j] = 1;
+return qemudDriveOpt(disk, 1);
+}
+j++;
+disk = disk-next;
+}
+qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ Requested boot device type %d, but no such device 
defined., type);
+return 0;
+}
 
 /*
  * Parses a libvirt XML definition of a guest, and populates the
@@ -1762,7 +1842,6 @@
 obj = xmlXPathEval(BAD_CAST /domain/devices/disk, ctxt);
 if ((obj != NULL)  (obj-type == XPATH_NODESET) 
 (obj-nodesetval != NULL)  (obj-nodesetval-nodeNr = 0)) {
-struct qemud_vm_disk_def *prev = NULL;
 for (i = 0; i  obj-nodesetval-nodeNr; i++) {
 struct qemud_vm_disk_def *disk = calloc(1, sizeof(*disk));
   

[libvirt] Changed mailing list subject line prefix

[Daniel P. Berrange]

I have just changed the mailing list subject line prefix from '[libvir] ' 
to '[libvirt] ', so if anyone is using that for filtering you'll need to
update...

Dan.
-- 
|: Red Hat, Engineering, Boston   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  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

[libvirt] Re: [Libvir] make syntax-check fails with bzr checkouts

[Soren Hansen]

On Wed, Apr 30, 2008 at 12:19:51PM +0200, Jim Meyering wrote:
 Unfortunately, the above no longer applies, due to upstream (gnulib)
 changes to deal with non-srcdir (aka VPATH) builds.  I updated libvirt
 from gnulib just yesterday, and will again, later today.

Here's the new patch that seems to do the trick:


=== modified file 'build-aux/vc-list-files'
--- old/build-aux/vc-list-files 2008-04-30 16:11:08 +
+++ new/build-aux/vc-list-files 2008-05-06 12:25:50 +
@@ -75,6 +75,9 @@
   eval exec git ls-files '$dir' $postprocess
 elif test -d .hg; then
   eval exec hg locate '$dir/*' $postprocess
+elif test -d .bzr; then
+  test $postprocess = ''  postprocess=| sed 's|^\./||'
+  eval exec bzr ls --versioned '$dir' $postprocess
 elif test -d CVS; then
   test $postprocess = ''  postprocess=| sed 's|^\./||'
   if test -x build-aux/cvsu; then


-- 
Soren Hansen   | 
Virtualisation specialist  | Ubuntu Server Team
Canonical Ltd. | http://www.ubuntu.com/


signature.asc
Description: Digital signature
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] Re: [Libvir] [PATCH] Add bus attribute to disk target definition

[Daniel P. Berrange]

On Tue, May 06, 2008 at 08:11:11PM +0200, Soren Hansen wrote:
 sorry for the delay. Here's the newest version of the patch which should
 address all the issues that have been raised so far.

Yes  no. It didn't address the redundant re-ordering of -drive parameters
when using boot=on, nor re-add the -boot param to make PXE work again. One
further complication is that QEMU 0.9.1 has -drive support but not the
extboot support, so boot=on can't be used there. It rather annoyingly
complains 

  qemu: unknowm parameter 'boot' in 
'file=/home/berrange/boot.iso,media=cdrom,boot=on'


 === modified file 'src/qemu_conf.c'
 --- old/src/qemu_conf.c   2008-04-30 12:30:55 +
 +++ new/src/qemu_conf.c   2008-05-06 17:58:44 +
 @@ -491,6 +491,8 @@
  *flags |= QEMUD_CMD_FLAG_KQEMU;
  if (strstr(help, -no-reboot))
  *flags |= QEMUD_CMD_FLAG_NO_REBOOT;
 +if (strstr(help, \n-drive))
 +*flags |= QEMUD_CMD_FLAG_DRIVE_OPT;

Turns out that this needed to also check for 'boot=on' availability.

 @@ -673,13 +676,28 @@
  goto error;
  }
  
 +if (!bus)
 +disk-bus = QEMUD_DISK_BUS_IDE;

This was giving floppy disks a bus of 'ide' which isn't technically
correct - they're really their own bus type - I reckon we should call
it 'fdc'.

 +else if (STREQ((const char *)bus, ide))
 +disk-bus = QEMUD_DISK_BUS_IDE;
 +else if (STREQ((const char *)bus, scsi))
 +disk-bus = QEMUD_DISK_BUS_SCSI;
 +else if (STREQ((const char *)bus, virtio))
 +disk-bus = QEMUD_DISK_BUS_VIRTIO;
 +else {
 +qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, 
 _(Invalid bus type: %s), bus);
 +goto error;
 +}
 +
  xmlFree(device);
  xmlFree(target);
  xmlFree(source);
 +xmlFree(bus);
  
  return 0;
  
   error:
 +xmlFree(bus);
  xmlFree(type);
  xmlFree(target);
  xmlFree(source);
 @@ -1373,6 +1391,68 @@
  return -1;
  }
  
 +static int qemudDiskCompare(const void *aptr, const void *bptr) {
 +struct qemud_vm_disk_def *a = (struct qemud_vm_disk_def *) aptr;
 +struct qemud_vm_disk_def *b = (struct qemud_vm_disk_def *) bptr;
 +if (a-device == b-device)
 +return virDiskNameToIndex(a-dst) - virDiskNameToIndex(b-dst);
 +else
 +return a-device - b-device;

Typo - this should be comparing the 'bus' field rather than 'device' to
get IDE, SCSI and VirtIO disks ordered correctly wrt to each other.

 +static const char *qemudBusIdToName(int busId) {
 +const char *busnames[] = { ide,
 +scsi,
 +virtio };
 +
 + if (busId = 0  busId  3)
 + return busnames[busId];
 + else
 + return 0;
 +}

This can be changed to make use of the GNULIB 'verify_true' function
to get a compile time check fo the array cardinality vs the enum
length.

 +static char *qemudDriveOpt(struct qemud_vm_disk_def *disk, int boot)
 +{
 +char opt[PATH_MAX];
 +
 +switch (disk-device) {
 +case QEMUD_DISK_CDROM:
 +snprintf(opt, PATH_MAX, file=%s,if=ide,media=cdrom%s,
 +  disk-src, boot ? ,boot=on : );
 +break;
 +case QEMUD_DISK_FLOPPY:
 +snprintf(opt, PATH_MAX, file=%s,if=floppy%s,
 +  disk-src, boot ? ,boot=on : );
 +break;
 +case QEMUD_DISK_DISK:
 +snprintf(opt, PATH_MAX, file=%s,if=%s%s,
 +  disk-src, qemudBusIdToName(disk-bus), boot ? 
 ,boot=on : );

This is missing the 'index=NNN' parameter so IDE disks are not getting
assigned to the correct bus/unit. eg if i define 'hda' and 'hdd' in the
XML the guest gets given 'hda' and 'hdb'.

 @@ -1775,13 +1854,20 @@
  goto error;
  }
  def-ndisks++;
 -disk-next = NULL;
  if (i == 0) {
 +disk-next = NULL;
  def-disks = disk;
  } else {
 -prev-next = disk;
 +struct qemud_vm_disk_def *ptr = def-disks;
 +while (ptr) {
 +if (!ptr-next || qemudDiskCompare(ptr-next, disk)  0) 
 {

The parameters to the compare function are inverted so the ordering
is being reversed.

 @@ -2110,6 +2196,7 @@
  struct qemud_vm_chr_def *parallel = vm-def-parallels;
  struct utsname ut;
  int disableKQEMU = 0;
 +int use_extboot = 0;
  
  /* Make sure the binary we are about to try exec'ing exists.
   * Technically we could catch the exec() failure, but that's
 @@ -2230,30 +2317,47 @@
  goto no_memory;
  }
  
 -for (i = 0 ; i  vm-def-os.nBootDevs ; i++) {
 -switch (vm-def-os.bootDevs[i]) {
 -case QEMUD_BOOT_CDROM:
 -boot[i] = 'd';
 -break;
 -case QEMUD_BOOT_FLOPPY:
 -boot[i] = 'a';
 -break;
 -case QEMUD_BOOT_DISK:
 -boot[i] = 

Re: [libvirt] Re: [Libvir] make syntax-check fails with bzr checkouts

[Jim Meyering]

Soren Hansen [EMAIL PROTECTED] wrote:
 On Wed, Apr 30, 2008 at 12:19:51PM +0200, Jim Meyering wrote:
 Unfortunately, the above no longer applies, due to upstream (gnulib)
 changes to deal with non-srcdir (aka VPATH) builds.  I updated libvirt
 from gnulib just yesterday, and will again, later today.

 Here's the new patch that seems to do the trick:

 === modified file 'build-aux/vc-list-files'
 --- old/build-aux/vc-list-files   2008-04-30 16:11:08 +
 +++ new/build-aux/vc-list-files   2008-05-06 12:25:50 +
 @@ -75,6 +75,9 @@
eval exec git ls-files '$dir' $postprocess
  elif test -d .hg; then
eval exec hg locate '$dir/*' $postprocess
 +elif test -d .bzr; then
 +  test $postprocess = ''  postprocess=| sed 's|^\./||'
 +  eval exec bzr ls --versioned '$dir' $postprocess
  elif test -d CVS; then
test $postprocess = ''  postprocess=| sed 's|^\./||'
if test -x build-aux/cvsu; then

Thanks.
That looks fine.  I've pushed it into gnulib (upstream),

http://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=commitdiff;h=176956aa54

and will apply here in libvirt shortly.

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

[libvirt] Re: [Libvir] [PATCH] sound support for qemu and xen

[Daniel P. Berrange]

On Tue, May 06, 2008 at 10:39:53AM -0400, Cole Robinson wrote:
 Cole Robinson wrote:
  The patch below adds xml support for the soundhw option to qemu
  and xen. 
 
 Third iteration. Changes include:
 
 - Rediffd against current code
 - Reading 'all' from a xend sexpr or xm config does the right thing.
 - Added test files.

Great, this looks good now

 - Addressed most of Jim's feedback. Unfortunately I could not reuse
 the qemu sound functions, as xen (on f8 at least) doesn't support
 the pcspk model, so the whitelist isn't the same between the two.

I don't think its a huge deal to have separate code for Xen vs QEMU.
The entire XML parser needs to be merged - just dealing with the sound
functions is such a minor deal its not worth it.

Regards,
Daniel.
-- 
|: Red Hat, Engineering, Boston   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  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

[libvirt] PATCH: Add disk bus attribute for Xen driver

[Daniel P. Berrange]

To complement soren's patch adding a bus attribute to the QEMU driver,
here is a minimal patch adding bus attribute to the Xen drivers. It merely
adds it on when generating the XML. It isn't making any attempt to interpret
it when creating a VM, since Xen does everything based off the disk node
name anyway its (currently) redundant.

The bus types supported  are 'xen' for paravirt disks, or 'ide' and 'scsi'
for HVM guests. 

NB, if setting up ide / scsi disks with HVM, XenD also sets up a parvirt
disk, but we don't attempt to express this duplication as its an underlying
impl detail.

Regards,
Daniel.

Index: src/xend_internal.c
===
RCS file: /data/cvs/libvirt/src/xend_internal.c,v
retrieving revision 1.183
diff -u -p -r1.183 xend_internal.c
--- src/xend_internal.c 30 Apr 2008 12:30:55 -  1.183
+++ src/xend_internal.c 7 May 2008 00:14:23 -
@@ -1759,6 +1759,7 @@ xend_parse_sexp_desc(virConnectPtr conn,
 const char *src = NULL;
 const char *dst = NULL;
 const char *mode = NULL;
+const char *bus = NULL;
 
 /* Again dealing with (vbd...) vs (tap ...) differences */
 if (sexpr_lookup(node, device/vbd)) {
@@ -1878,7 +1879,16 @@ xend_parse_sexp_desc(virConnectPtr conn,
 /* This case is the cdrom device only */
 virBufferAddLit(buf, disk device='cdrom'\n);
 }
-virBufferVSprintf(buf,   target dev='%s'/\n, dst);
+
+if (STRPREFIX(dst, xvd) || !hvm) {
+bus = xen;
+} else if (STRPREFIX(dst, sd)) {
+bus = scsi;
+} else {
+bus = ide;
+}
+virBufferVSprintf(buf,   target dev='%s' bus='%s'/\n,
+  dst, bus);
 
 
 /* XXX should we force mode == r, if cdrom==1, or assume
@@ -1967,14 +1977,14 @@ xend_parse_sexp_desc(virConnectPtr conn,
 if ((tmp != NULL)  (tmp[0] != 0)) {
 virBufferAddLit(buf, disk type='file' device='floppy'\n);
 virBufferVSprintf(buf,   source file='%s'/\n, tmp);
-virBufferAddLit(buf,   target dev='fda'/\n);
+virBufferAddLit(buf,   target dev='fda' bus='fdc'/\n);
 virBufferAddLit(buf, /disk\n);
 }
 tmp = sexpr_node(root, domain/image/hvm/fdb);
 if ((tmp != NULL)  (tmp[0] != 0)) {
 virBufferAddLit(buf, disk type='file' device='floppy'\n);
 virBufferVSprintf(buf,   source file='%s'/\n, tmp);
-virBufferAddLit(buf,   target dev='fdb'/\n);
+virBufferAddLit(buf,   target dev='fdb' bus='fdc'/\n);
 virBufferAddLit(buf, /disk\n);
 }
 
@@ -1985,7 +1995,7 @@ xend_parse_sexp_desc(virConnectPtr conn,
 virBufferAddLit(buf, disk type='file' 
device='cdrom'\n);
 virBufferAddLit(buf,   driver name='file'/\n);
 virBufferVSprintf(buf,   source file='%s'/\n, tmp);
-virBufferAddLit(buf,   target dev='hdc'/\n);
+virBufferAddLit(buf,   target dev='hdc' bus='ide'/\n);
 virBufferAddLit(buf,   readonly/\n);
 virBufferAddLit(buf, /disk\n);
 }
Index: src/xm_internal.c
===
RCS file: /data/cvs/libvirt/src/xm_internal.c,v
retrieving revision 1.74
diff -u -p -r1.74 xm_internal.c
--- src/xm_internal.c   30 Apr 2008 12:30:55 -  1.74
+++ src/xm_internal.c   7 May 2008 00:14:26 -
@@ -733,6 +733,7 @@ char *xenXMDomainFormatXML(virConnectPtr
 char *head;
 char *offset;
 char *tmp, *tmp1;
+const char *bus;
 
 if ((list-type != VIR_CONF_STRING) || (list-str == NULL))
 goto skipdisk;
@@ -805,6 +806,14 @@ char *xenXMDomainFormatXML(virConnectPtr
 tmp[0] = '\0';
 }
 
+if (STRPREFIX(dev, xvd) || !hvm) {
+bus = xen;
+} else if (STRPREFIX(dev, sd)) {
+bus = scsi;
+} else {
+bus = ide;
+}
+
 virBufferVSprintf(buf, disk type='%s' device='%s'\n,
   block ? block : file,
   cdrom ? cdrom : disk);
@@ -814,7 +823,7 @@ char *xenXMDomainFormatXML(virConnectPtr
 virBufferVSprintf(buf,   driver name='%s'/\n, 
drvName);
 if (src[0])
 virBufferVSprintf(buf,   source %s='%s'/\n, block ? 
dev : file, src);
-virBufferVSprintf(buf,   target dev='%s'/\n, dev);
+virBufferVSprintf(buf,   target dev='%s' bus='%s'/\n, 
dev, bus);
 if (!strcmp(head, r) ||
 !strcmp(head, ro))
 virBufferAddLit(buf,