- Devel - libvirt List Archives

[libvirt] [PATCH] Fix sending/receiving of FDs when stream returns EAGAIN
by Daniel P. Berrange 04 Nov '11

04 Nov '11

From: "Daniel P. Berrange" <berrange(a)redhat.com> The code calling sendfd/recvfd was mistakenly assuming those calls would never block. They can in fact return EAGAIN and this is causing us to drop the client connection when blocking ocurrs while sending/receiving FDs. Fixing this is a little hairy on the incoming side, since at the point where we see the EAGAIN, we already thought we had finished receiving all data for the packet. So we play a little trick to reset bufferOffset again and go back into polling for more data. * src/rpc/virnetsocket.c, src/rpc/virnetsocket.h: Update virNetSocketSendFD/RecvFD to return 0 on EAGAIN, or 1 on success * src/rpc/virnetclient.c: Move decoding of header & fds out of virNetClientCallDispatch and into virNetClientIOHandleInput. Handling blocking when sending/receiving FDs * src/rpc/virnetmessage.h: Add a 'donefds' field to track how many FDs we've sent / received * src/rpc/virnetserverclient.c: Handling blocking when sending/receiving FDs --- src/rpc/virnetclient.c | 79 ++++++++++++++++++++++++++++-------------- src/rpc/virnetmessage.h | 1 + src/rpc/virnetserverclient.c | 62 ++++++++++++++++++++++++--------- src/rpc/virnetsocket.c | 34 +++++++++++++----- src/rpc/virnetsocket.h | 2 +- 5 files changed, 125 insertions(+), 53 deletions(-) diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index 2b5f67c..4b7d4a9 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -694,10 +694,6 @@ static int virNetClientCallDispatchStream(virNetClientPtr client) static int virNetClientCallDispatch(virNetClientPtr client) { - size_t i; - if (virNetMessageDecodeHeader(&client->msg) < 0) - return -1; - PROBE(RPC_CLIENT_MSG_RX, "client=%p len=%zu prog=%u vers=%u proc=%u type=%u status=%u serial=%u", client, client->msg.bufferLength, @@ -706,15 +702,7 @@ virNetClientCallDispatch(virNetClientPtr client) switch (client->msg.header.type) { case VIR_NET_REPLY: /* Normal RPC replies */ - return virNetClientCallDispatchReply(client); - case VIR_NET_REPLY_WITH_FDS: /* Normal RPC replies with FDs */ - if (virNetMessageDecodeNumFDs(&client->msg) < 0) - return -1; - for (i = 0 ; i < client->msg.nfds ; i++) { - if ((client->msg.fds[i] = virNetSocketRecvFD(client->sock)) < 0) - return -1; - } return virNetClientCallDispatchReply(client); case VIR_NET_MESSAGE: /* Async notifications */ @@ -737,22 +725,29 @@ static ssize_t virNetClientIOWriteMessage(virNetClientPtr client, virNetClientCallPtr thecall) { - ssize_t ret; + ssize_t ret = 0; - ret = virNetSocketWrite(client->sock, - thecall->msg->buffer + thecall->msg->bufferOffset, - thecall->msg->bufferLength - thecall->msg->bufferOffset); - if (ret <= 0) - return ret; + if (thecall->msg->bufferOffset < thecall->msg->bufferLength) { + ret = virNetSocketWrite(client->sock, + thecall->msg->buffer + thecall->msg->bufferOffset, + thecall->msg->bufferLength - thecall->msg->bufferOffset); + if (ret <= 0) + return ret; - thecall->msg->bufferOffset += ret; + thecall->msg->bufferOffset += ret; + } if (thecall->msg->bufferOffset == thecall->msg->bufferLength) { size_t i; - for (i = 0 ; i < thecall->msg->nfds ; i++) { - if (virNetSocketSendFD(client->sock, thecall->msg->fds[i]) < 0) + for (i = thecall->msg->donefds ; i < thecall->msg->nfds ; i++) { + int rv; + if ((rv = virNetSocketSendFD(client->sock, thecall->msg->fds[i])) < 0) return -1; + if (rv == 0) /* Blocking */ + return 0; + thecall->msg->donefds++; } + thecall->msg->donefds = 0; thecall->msg->bufferOffset = thecall->msg->bufferLength = 0; if (thecall->expectReply) thecall->mode = VIR_NET_CLIENT_MODE_WAIT_RX; @@ -821,12 +816,16 @@ virNetClientIOHandleInput(virNetClientPtr client) * EAGAIN */ for (;;) { - ssize_t ret = virNetClientIOReadMessage(client); + ssize_t ret; - if (ret < 0) - return -1; - if (ret == 0) - return 0; /* Blocking on read */ + if (client->msg.nfds == 0) { + ret = virNetClientIOReadMessage(client); + + if (ret < 0) + return -1; + if (ret == 0) + return 0; /* Blocking on read */ + } /* Check for completion of our goal */ if (client->msg.bufferOffset == client->msg.bufferLength) { @@ -842,6 +841,33 @@ virNetClientIOHandleInput(virNetClientPtr client) * next iteration. */ } else { + if (virNetMessageDecodeHeader(&client->msg) < 0) + return -1; + + if (client->msg.header.type == VIR_NET_REPLY_WITH_FDS) { + size_t i; + if (virNetMessageDecodeNumFDs(&client->msg) < 0) + return -1; + + for (i = client->msg.donefds ; i < client->msg.nfds ; i++) { + int rv; + if ((rv = virNetSocketRecvFD(client->sock, &(client->msg.fds[i]))) < 0) + return -1; + if (rv == 0) /* Blocking */ + break; + client->msg.donefds++; + } + + if (client->msg.donefds < client->msg.nfds) { + /* Because DecodeHeader/NumFDs reset bufferOffset, we + * put it back to what it was, so everything works + * again next time we run this method + */ + client->msg.bufferOffset = client->msg.bufferLength; + return 0; /* Blocking on more fds */ + } + } + ret = virNetClientCallDispatch(client); client->msg.bufferOffset = client->msg.bufferLength = 0; /* @@ -1257,6 +1283,7 @@ int virNetClientSend(virNetClientPtr client, goto cleanup; } + msg->donefds = 0; if (msg->bufferLength) call->mode = VIR_NET_CLIENT_MODE_WAIT_TX; else diff --git a/src/rpc/virnetmessage.h b/src/rpc/virnetmessage.h index ad63409..c54e7c6 100644 --- a/src/rpc/virnetmessage.h +++ b/src/rpc/virnetmessage.h @@ -48,6 +48,7 @@ struct _virNetMessage { size_t nfds; int *fds; + size_t donefds; virNetMessagePtr next; }; diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index 2f5ae8f..cf97b58 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -771,9 +771,11 @@ static ssize_t virNetServerClientRead(virNetServerClientPtr client) static void virNetServerClientDispatchRead(virNetServerClientPtr client) { readmore: - if (virNetServerClientRead(client) < 0) { - client->wantClose = true; - return; /* Error */ + if (client->rx->nfds == 0) { + if (virNetServerClientRead(client) < 0) { + client->wantClose = true; + return; /* Error */ + } } if (client->rx->bufferOffset < client->rx->bufferLength) @@ -794,7 +796,7 @@ readmore: goto readmore; } else { /* Grab the completed message */ - virNetMessagePtr msg = virNetMessageQueueServe(&client->rx); + virNetMessagePtr msg = client->rx; virNetServerClientFilterPtr filter; size_t i; @@ -805,20 +807,40 @@ readmore: return; } + /* Now figure out if we need to read more data to get some + * file descriptors */ if (msg->header.type == VIR_NET_CALL_WITH_FDS && virNetMessageDecodeNumFDs(msg) < 0) { virNetMessageFree(msg); client->wantClose = true; - return; + return; /* Error */ } - for (i = 0 ; i < msg->nfds ; i++) { - if ((msg->fds[i] = virNetSocketRecvFD(client->sock)) < 0) { + + /* Try getting the file descriptors (may fail if blocking) */ + for (i = msg->donefds ; i < msg->nfds ; i++) { + int rv; + if ((rv = virNetSocketRecvFD(client->sock, &(msg->fds[i]))) < 0) { virNetMessageFree(msg); client->wantClose = true; return; } + if (rv == 0) /* Blocking */ + break; + msg->donefds++; + } + + /* Need to poll() until FDs arrive */ + if (msg->donefds < msg->nfds) { + /* Because DecodeHeader/NumFDs reset bufferOffset, we + * put it back to what it was, so everything works + * again next time we run this method + */ + client->rx->bufferOffset = client->rx->bufferLength; + return; } + /* Definitely finished reading, so remove from queue */ + virNetMessageQueueServe(&client->rx); PROBE(RPC_SERVER_CLIENT_MSG_RX, "client=%p len=%zu prog=%u vers=%u proc=%u type=%u status=%u serial=%u", client, msg->bufferLength, @@ -912,25 +934,30 @@ static void virNetServerClientDispatchWrite(virNetServerClientPtr client) { while (client->tx) { - ssize_t ret; - - ret = virNetServerClientWrite(client); - if (ret < 0) { - client->wantClose = true; - return; + if (client->tx->bufferOffset < client->tx->bufferLength) { + ssize_t ret; + ret = virNetServerClientWrite(client); + if (ret < 0) { + client->wantClose = true; + return; + } + if (ret == 0) + return; /* Would block on write EAGAIN */ } - if (ret == 0) - return; /* Would block on write EAGAIN */ if (client->tx->bufferOffset == client->tx->bufferLength) { virNetMessagePtr msg; size_t i; - for (i = 0 ; i < client->tx->nfds ; i++) { - if (virNetSocketSendFD(client->sock, client->tx->fds[i]) < 0) { + for (i = client->tx->donefds ; i < client->tx->nfds ; i++) { + int rv; + if ((rv = virNetSocketSendFD(client->sock, client->tx->fds[i])) < 0) { client->wantClose = true; return; } + if (rv == 0) /* Blocking */ + return; + client->tx->donefds++; } #if HAVE_SASL @@ -1041,6 +1068,7 @@ int virNetServerClientSendMessage(virNetServerClientPtr client, msg->bufferLength, msg->bufferOffset); virNetServerClientLock(client); + msg->donefds = 0; if (client->sock && !client->wantClose) { PROBE(RPC_SERVER_CLIENT_MSG_TX_QUEUE, "client=%p len=%zu prog=%u vers=%u proc=%u type=%u status=%u serial=%u", diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index d832c53..4517d16 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -1142,6 +1142,9 @@ ssize_t virNetSocketWrite(virNetSocketPtr sock, const char *buf, size_t len) } +/* + * Returns 1 if an FD was sent, 0 if it would block, -1 on error + */ int virNetSocketSendFD(virNetSocketPtr sock, int fd) { int ret = -1; @@ -1154,12 +1157,15 @@ int virNetSocketSendFD(virNetSocketPtr sock, int fd) PROBE(RPC_SOCKET_SEND_FD, "sock=%p fd=%d", sock, fd); if (sendfd(sock->fd, fd) < 0) { - virReportSystemError(errno, - _("Failed to send file descriptor %d"), - fd); + if (errno == EAGAIN) + ret = 0; + else + virReportSystemError(errno, + _("Failed to send file descriptor %d"), + fd); goto cleanup; } - ret = 0; + ret = 1; cleanup: virMutexUnlock(&sock->lock); @@ -1167,9 +1173,15 @@ cleanup: } -int virNetSocketRecvFD(virNetSocketPtr sock) +/* + * Returns 1 if an FD was read, 0 if it would block, -1 on error + */ +int virNetSocketRecvFD(virNetSocketPtr sock, int *fd) { int ret = -1; + + *fd = -1; + if (!virNetSocketHasPassFD(sock)) { virNetError(VIR_ERR_INTERNAL_ERROR, _("Receiving file descriptors is not supported on this socket")); @@ -1177,13 +1189,17 @@ int virNetSocketRecvFD(virNetSocketPtr sock) } virMutexLock(&sock->lock); - if ((ret = recvfd(sock->fd, O_CLOEXEC)) < 0) { - virReportSystemError(errno, "%s", - _("Failed to recv file descriptor")); + if ((*fd = recvfd(sock->fd, O_CLOEXEC)) < 0) { + if (errno == EAGAIN) + ret = 0; + else + virReportSystemError(errno, "%s", + _("Failed to recv file descriptor")); goto cleanup; } PROBE(RPC_SOCKET_RECV_FD, - "sock=%p fd=%d", sock, ret); + "sock=%p fd=%d", sock, *fd); + ret = 1; cleanup: virMutexUnlock(&sock->lock); diff --git a/src/rpc/virnetsocket.h b/src/rpc/virnetsocket.h index 13cbb14..e444aef 100644 --- a/src/rpc/virnetsocket.h +++ b/src/rpc/virnetsocket.h @@ -97,7 +97,7 @@ ssize_t virNetSocketRead(virNetSocketPtr sock, char *buf, size_t len); ssize_t virNetSocketWrite(virNetSocketPtr sock, const char *buf, size_t len); int virNetSocketSendFD(virNetSocketPtr sock, int fd); -int virNetSocketRecvFD(virNetSocketPtr sock); +int virNetSocketRecvFD(virNetSocketPtr sock, int *fd); void virNetSocketSetTLSSession(virNetSocketPtr sock, virNetTLSSessionPtr sess); -- 1.7.6.4

2 1

[libvirt] [PATCH] remote: fix mingw32 build
by Laine Stump 04 Nov '11

04 Nov '11

tty is initialized, and later set in code that is compiled for all platforms, but is only used in a section that's inside #ifndef WIN32. --- src/remote/remote_driver.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index ea7fb24..f3b8ad5 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -377,7 +377,7 @@ doRemoteOpen (virConnectPtr conn, */ char *name = NULL, *command = NULL, *sockname = NULL, *netcat = NULL; char *port = NULL, *authtype = NULL, *username = NULL; - bool sanity = true, verify = true, tty = true; + bool sanity = true, verify = true, tty ATTRIBUTE_UNUSED = true; char *pkipath = NULL, *keyfile = NULL; /* Return code from this function, and the private data. */ -- 1.7.7

2 1

[libvirt] [PATCH] lxc: avoid use-after-free
by Eric Blake 04 Nov '11

04 Nov '11

I got this weird failure: error: Failed to start domain simple error: internal error cannot mix caller fds with blocking execution and tracked it down to a use-after-free - virCommandSetOutputFD was storing the address of a stack-local variable, which then went out of scope before the virCommandRun that dereferenced it. Bug introduced in commit 451cfd05 (0.9.2). * src/lxc/lxc_driver.c (lxcBuildControllerCmd): Move log fd registration... (lxcVmStart): ...to caller. --- I have no idea how danpb got so lucky in being able to test recent lxc addtions, given the fact that booting an LXC domain has basically been broken for several months now, depending on whether the compiler happened to smash the stack variable in question. src/lxc/lxc_driver.c | 7 +++---- 1 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index d6e5e20..37092bc 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -1449,7 +1449,6 @@ lxcBuildControllerCmd(lxc_driver_t *driver, char **veths, int *ttyFDs, size_t nttyFDs, - int logfile, int handshakefd) { size_t i; @@ -1524,8 +1523,6 @@ lxcBuildControllerCmd(lxc_driver_t *driver, } virCommandPreserveFD(cmd, handshakefd); - virCommandSetOutputFD(cmd, &logfile); - virCommandSetErrorFD(cmd, &logfile); return cmd; cleanup: @@ -1747,8 +1744,10 @@ static int lxcVmStart(virConnectPtr conn, vm, nveths, veths, ttyFDs, nttyFDs, - logfd, handshakefds[1]))) + handshakefds[1]))) goto cleanup; + virCommandSetOutputFD(cmd, &logfd); + virCommandSetErrorFD(cmd, &logfd); /* Log timestamp */ if ((timestamp = virTimestamp()) == NULL) { -- 1.7.4.4

2 2

[libvirt] [PATCH] Fix naming of constant for disk event
by Daniel P. Berrange 04 Nov '11

04 Nov '11

From: "Daniel P. Berrange" <berrange(a)redhat.com> All constants related to events should have a prefix of VIR_DOMAIN_EVENT_ * include/libvirt/libvirt.h.in, src/qemu/qemu_domain.c: Rename VIR_DOMAIN_DISK_CHANGE_MISSING_ON_START to VIR_DOMAIN_EVENT_DISK_CHANGE_MISSING_ON_START --- include/libvirt/libvirt.h.in | 4 ++-- src/qemu/qemu_domain.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 7181f62..aa320b6 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -3069,12 +3069,12 @@ typedef void (*virConnectDomainEventBlockJobCallback)(virConnectPtr conn, void *opaque); /** - * virConnectDomainEventDisChangeReason: + * virConnectDomainEventDiskChangeReason: * * The reason describing why this callback is called */ typedef enum { - VIR_DOMAIN_DISK_CHANGE_MISSING_ON_START = 0, /* oldSrcPath is set */ + VIR_DOMAIN_EVENT_DISK_CHANGE_MISSING_ON_START = 0, /* oldSrcPath is set */ } virConnectDomainEventDiskChangeReason; /** diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 5295783..3e755d7 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1652,7 +1652,7 @@ qemuDomainCheckDiskPresence(struct qemud_driver *driver, disk->dst, vm->def->name, uuid, disk->src); event = virDomainEventDiskChangeNewFromObj(vm, disk->src, NULL, disk->info.alias, - VIR_DOMAIN_DISK_CHANGE_MISSING_ON_START); + VIR_DOMAIN_EVENT_DISK_CHANGE_MISSING_ON_START); if (event) qemuDomainEventQueue(driver, event); -- 1.7.6.4

3 2

[libvirt] libvirt-guests - migrate/relocate on stop feature request
by Maurits van de Lande 04 Nov '11

04 Nov '11

Hello I'm using KVM in a clustered environment. When a host server shuts down or reboots the VM's should migrate to another cluster node. Currently I'm using a custom script for a two node cluster. It would be very nice if this feature could be incorporated into the libvirt-guests script. Or if a host could be specified as a relocation host. Is there any chance for this feature to be added to the script? Sincerely, Maurits van de Lande

2 3

[libvirt] [PATCH] conf: Don't free uninitialized pointer
by Jiri Denemark 04 Nov '11

04 Nov '11

This causes libvirtd to crash when both <boot dev='...'/> and <boot order='...'/> are used in one domain XML. Introduced by 5fa3d775a9f8cdb3423373eb084219aaf778df11 --- src/conf/domain_conf.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 238edfd..a85f837 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6430,7 +6430,8 @@ virDomainDefParseBootXML(xmlXPathContextPtr ctxt, { xmlNodePtr *nodes = NULL; int i, n; - char *bootstr, *useserial; + char *bootstr; + char *useserial = NULL; int ret = -1; unsigned long deviceBoot, serialPorts; -- 1.7.7.2

3 3

[libvirt] can't start domain with a corrupted disk attatched
by lvroyce 04 Nov '11

04 Nov '11

Hi all, I came across below issue when testing: 1.make a volume and attach it to a domain A 2.unplug the vg from the host in order to emulating a volume failure 3.start domain A(failed) In step 3 can't start domainA . because can't find disk listed in xml when create the Domain. I'm not sure if it is reasonable. In common sense, we can still start our system even if we have a corrupt data disk .And also ,if in data center we carelessly attatch a corrupt volumn to all the guest, it will result in all guests fail to boot . I suggest to automatically detach a disk if it can't be found and just give out a warning.Please let me know your opinion about if it is a bug or a feature.Thanks.

5 4

[libvirt] [RFC PATCH 00/10] use cpuset to manage numa
by Hu Tao 03 Nov '11

03 Nov '11

The goal of this series is to use cpuset to manage numa. Currently numa tuning is achieved by libnuma, the disadvantage is numa parameters can not be modified when the domain is running. By using cpuset, we can do this and more: - a new command, numatune, to get/set numa parameters - cpuset can migrate pages to new nodes if a domain's numa nodes is changed when running This is a draft for early reviews. Comments are welcome. TODO: - add --migrate option to numatune - docs for numatune Hu Tao (10): enable cgroup cpuset by default Add functions to set/get cpuset parameters use cpuset to manage numa remove dependence on libnuma new numa parameters add new API virDomain{G,S}etNumaParameters Implement main entries of virDomain{G,S}etNumaParameters Add virDomain{G,S}etNumaParameters support to the remote driver Implement virDomain{G,S}etNumaParameters for the qemu driver add new command numatune to virsh daemon/remote.c | 63 +++++++++ include/libvirt/libvirt.h.in | 23 +++ python/generator.py | 2 + src/conf/domain_conf.c | 35 ++--- src/conf/domain_conf.h | 3 +- src/driver.h | 15 ++ src/libvirt.c | 106 +++++++++++++++ src/libvirt_private.syms | 6 + src/libvirt_public.syms | 6 + src/qemu/qemu.conf | 5 +- src/qemu/qemu_cgroup.c | 37 +++++ src/qemu/qemu_conf.c | 3 +- src/qemu/qemu_driver.c | 307 ++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_process.c | 111 --------------- src/remote/remote_driver.c | 50 +++++++ src/remote/remote_protocol.x | 25 ++++- src/remote_protocol-structs | 16 +++ src/util/cgroup.c | 80 +++++++++++ src/util/cgroup.h | 7 + tools/virsh.c | 187 +++++++++++++++++++++++++ 20 files changed, 950 insertions(+), 137 deletions(-) -- 1.7.3.1

2 13

[libvirt] [PATCH] build: fix deep VPATH builds
by Eric Blake 03 Nov '11

03 Nov '11

I ran into the following build failure: $ mkdir -p build1 build2/a/very/deep/hierarcy $ cd build2/a/very/deep/hierarcy $ ../../../../../configure && make $ cd ../../../../build1 $ ../configure && make ... ../../src/remote/remote_protocol.c:7:55: fatal error: ../../../../../src/remote/remote_protocol.h: No such file or directory Turns out that we were sometimes generating the remote_protocol.c file with information from the VPATH build, which is bad, since any file shipped in the tarball should be idempotent no matter how deep the VPATH build tree that created it. * src/rpc/genprotocol.pl: Don't embed VPATH into generated file. --- Pushing under the build-breaker rule. And yes, I really did create a directory by that name, while testing the earlier patches for the testsuite failure to create a UNIX socket in a deep location. :) src/rpc/genprotocol.pl | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/rpc/genprotocol.pl b/src/rpc/genprotocol.pl index 7124e5c..166508b 100755 --- a/src/rpc/genprotocol.pl +++ b/src/rpc/genprotocol.pl @@ -59,7 +59,7 @@ while (<RPCGEN>) { s/xdr_u_quad_t/xdr_uint64_t/g; s/xdr_quad_t/xdr_int64_t/g; s/(?<!IXDR_GET_INT32 )IXDR_GET_LONG/IXDR_GET_INT32/g; - s,#include "remote/remote_protocol\.h",#include "remote_protocol.h",; + s,#include ".*remote/remote_protocol\.h",#include "remote_protocol.h",; if (m/^}/) { $in_function = 0; -- 1.7.4.4

1 0

[libvirt] [PATCH 1/2] Fix order of disks and controllers
by Jiri Denemark 03 Nov '11

03 Nov '11

Commit 2d6adabd53c8f1858191d521dc9b4948d1205955 replaced qsorting disk and controller devices with inserting them at the right position. That was to fix unnecessary reordering of devices. However, when parsing domain XML devices are just taken in the order in which they appear in the XML since. Use the correct insertion algorithm to honor device target. --- src/conf/domain_conf.c | 4 +- .../qemuxml2argvdata/qemuxml2argv-boot-order.args | 35 ++++++++++++------- tests/qemuxml2argvdata/qemuxml2argv-boot-order.xml | 14 ++++---- tests/xmconfigdata/test-escape-paths.cfg | 2 +- tests/xmconfigdata/test-escape-paths.xml | 10 +++--- 5 files changed, 37 insertions(+), 28 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b406202..48ed7eb 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5883,7 +5883,7 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, if (!disk) goto error; - def->disks[def->ndisks++] = disk; + virDomainDiskInsertPreAlloced(def, disk); } VIR_FREE(nodes); @@ -5899,7 +5899,7 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, if (!controller) goto error; - def->controllers[def->ncontrollers++] = controller; + virDomainControllerInsertPreAlloced(def, controller); } VIR_FREE(nodes); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-boot-order.args b/tests/qemuxml2argvdata/qemuxml2argv-boot-order.args index 23249f3..14367b1 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-boot-order.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-boot-order.args @@ -1,13 +1,22 @@ -LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ -pc -m 214 -smp 1 -nographic -nodefaults -monitor unix:/tmp/test-monitor,server,\ -nowait -no-acpi -drive file=/dev/HostVG/QEMUGuest1,if=none,id=drive-ide0-0-0 \ --device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 -drive \ -file=sheepdog:example.org:6000:image,if=none,id=drive-virtio-disk0 -device \ -virtio-blk-pci,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,id=virtio-disk0,\ -bootindex=3 -drive file=/root/boot.iso,if=none,media=cdrom,id=drive-ide0-1-0 \ --device ide-drive,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0,\ -bootindex=1 -drive file=/dev/null,if=none,id=drive-fdc0-0-1 -global \ -isa-fdc.driveB=drive-fdc0-0-1 -global isa-fdc.bootindexB=4 -device \ -virtio-net-pci,vlan=0,id=net0,mac=00:11:22:33:44:55,bus=pci.0,addr=0x3,\ -bootindex=2 -net user,vlan=0,name=hostnet0 -usb -device virtio-balloon-pci,\ -id=balloon0,bus=pci.0,addr=0x5 +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu \ +-S \ +-M pc \ +-m 214 \ +-smp 1 \ +-nographic \ +-nodefaults \ +-monitor unix:/tmp/test-monitor,server,nowait \ +-no-acpi \ +-drive file=/dev/HostVG/QEMUGuest1,if=none,id=drive-ide0-0-0 \ +-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ +-drive file=/root/boot.iso,if=none,media=cdrom,id=drive-ide0-1-0 \ +-device ide-drive,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0,bootindex=1 \ +-drive file=sheepdog:example.org:6000:image,if=none,id=drive-virtio-disk0 \ +-device virtio-blk-pci,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=3 \ +-drive file=/dev/null,if=none,id=drive-fdc0-0-1 \ +-global isa-fdc.driveB=drive-fdc0-0-1 \ +-global isa-fdc.bootindexB=4 \ +-device virtio-net-pci,vlan=0,id=net0,mac=00:11:22:33:44:55,bus=pci.0,addr=0x3,bootindex=2 \ +-net user,vlan=0,name=hostnet0 \ +-usb \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x5 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-boot-order.xml b/tests/qemuxml2argvdata/qemuxml2argv-boot-order.xml index ba8a9b2..0022c92 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-boot-order.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-boot-order.xml @@ -18,6 +18,13 @@ <target dev='hda' bus='ide'/> <address type='drive' controller='0' bus='0' unit='0'/> </disk> + <disk type='file' device='cdrom'> + <source file='/root/boot.iso'/> + <target dev='hdc' bus='ide'/> + <boot order='1'/> + <readonly/> + <address type='drive' controller='0' bus='1' unit='0'/> + </disk> <disk type='network' device='disk'> <driver name='qemu' type='raw'/> <source protocol='sheepdog' name='image'> @@ -26,13 +33,6 @@ <target dev='vda' bus='virtio'/> <boot order='3'/> </disk> - <disk type='file' device='cdrom'> - <source file='/root/boot.iso'/> - <target dev='hdc' bus='ide'/> - <boot order='1'/> - <readonly/> - <address type='drive' controller='0' bus='1' unit='0'/> - </disk> <disk type='file' device='floppy'> <driver name='qemu' type='raw'/> <source file='/dev/null'/> diff --git a/tests/xmconfigdata/test-escape-paths.cfg b/tests/xmconfigdata/test-escape-paths.cfg index e3e6db9..13be2a0 100644 --- a/tests/xmconfigdata/test-escape-paths.cfg +++ b/tests/xmconfigdata/test-escape-paths.cfg @@ -19,7 +19,7 @@ vnc = 1 vncunused = 1 vnclisten = "127.0.0.1" vncpasswd = "123poi" -disk = [ "phy:/dev/HostVG/XenGuest2,hda,w", "file:/root/boot.iso&test,hdc:cdrom,r", """phy:/dev/HostVG/XenGuest'",hdb,w""" ] +disk = [ "phy:/dev/HostVG/XenGuest2,hda,w", """phy:/dev/HostVG/XenGuest'",hdb,w""", "file:/root/boot.iso&test,hdc:cdrom,r" ] vif = [ "mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=e1000,type=ioemu" ] parallel = "none" serial = "none" diff --git a/tests/xmconfigdata/test-escape-paths.xml b/tests/xmconfigdata/test-escape-paths.xml index 13e6e29..9eaf90c 100644 --- a/tests/xmconfigdata/test-escape-paths.xml +++ b/tests/xmconfigdata/test-escape-paths.xml @@ -25,17 +25,17 @@ <source dev='/dev/HostVG/XenGuest2'/> <target dev='hda' bus='ide'/> </disk> + <disk type='block' device='disk'> + <driver name='phy'/> + <source dev='/dev/HostVG/XenGuest'"'/> + <target dev='hdb' bus='ide'/> + </disk> <disk type='file' device='cdrom'> <driver name='file'/> <source file='/root/boot.iso&test'/> <target dev='hdc' bus='ide'/> <readonly/> </disk> - <disk type='block' device='disk'> - <driver name='phy'/> - <source dev='/dev/HostVG/XenGuest'"'/> - <target dev='hdb' bus='ide'/> - </disk> <interface type='bridge'> <mac address='00:16:3e:66:92:9c'/> <source bridge='xenbr1'/> -- 1.7.5.3

4 21