Re: [libvirt] [PATCH 2/3] snapshot: save domain description with snapshot
Hello Eric, Am Samstag 13 August 2011 00:08:11 schrieb Eric Blake: On 04/12/2011 12:16 AM, Philipp Hahn wrote: Save the domain description with the XML snapshot data. TODOs: - XML file is no longer nicely indented Cosmetic, and can be fixed later. - Fix esx driver - Fix vbox driver Do these need to save domain xml state in the first place? They aren't using libvirt to track domain state in the first time, but call out to the hypervisor for everything. And if the hypervisor is already doing a good job of reverting across configuration changes, then it doesn't hurt if they continue to use just domain/uuid instead of full domain in the snapshot output that libvirt generates on virDomainSnapshotGetXMLDesc. I don't have access to any ESX system, so I couldn't check. At least with our (very) old VMWare Server when doing a snapshot, the configuration is saved, so on revert you get the old configuration again. That difference was actually what got us to implement this for Qemu as well. VBox I didn't check: We're using it for another project I'm currently not working on, but there libvirt isn't used to manage it. @@ -8694,9 +8705,17 @@ char *virDomainSnapshotDefFormat(char *domain_uuid, } virBufferVSprintf(buf, creationTime%ld/creationTime\n, def-creationTime); -virBufferAddLit(buf, domain\n); -virBufferVSprintf(buf, uuid%s/uuid\n, domain_uuid); -virBufferAddLit(buf, /domain\n); +if (def-dom != NULL) { +xml = virDomainDefFormat(def-dom, VIR_DOMAIN_XML_INACTIVE | VIR_DOMAIN_XML_SECURE); Security hole. You cannot blindly add VIR_DOMAIN_XML_SECURE if this is destined to external output, rather, it has to be passed in from the user's flags, and libvirt.c has to validate that virDomainSnapshotGetXMLDesc rejects the flag on read-only connections. Yes, but for a PoC that was the easiest thing to do. Glad you spotted that. Sincerely Philipp Hahn -- Philipp Hahn Open Source Software Engineer h...@univention.de Univention GmbHLinux for Your Businessfon: +49 421 22 232- 0 Mary-Somerville-Str.1 D-28359 Bremen fax: +49 421 22 232-99 http://www.univention.de/ signature.asc Description: This is a digitally signed message part. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] daemon: Fix regression of libvirtd reloading support
This is introduced by commit df0b57a95a, which forgot to add signal handler for SIGHUP. A simple reproduce method: 1) Create a domain XML under /etc/libvirt/qemu 2) % kill -SIGHUP $(pidof libvirtd) 3) % virsh list --all (the new created domain XML is not listed) --- daemon/libvirtd.c | 13 + 1 files changed, 13 insertions(+), 0 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index b866a01..3e0159b 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -1102,6 +1102,17 @@ static void daemonShutdownHandler(virNetServerPtr srv, virNetServerQuit(srv); } +static void daemonReloadHandler(virNetServerPtr srv ATTRIBUTE_UNUSED, +siginfo_t *sig ATTRIBUTE_UNUSED, +void *opaque ATTRIBUTE_UNUSED) +{ +VIR_INFO(_(Reloading configuration on SIGHUP)); +virHookCall(VIR_HOOK_DRIVER_DAEMON, -, +VIR_HOOK_DAEMON_OP_RELOAD, SIGHUP, SIGHUP, NULL); +if (virStateReload() 0) +VIR_WARN(Error while reloading drivers); +} + static int daemonSetupSignals(virNetServerPtr srv) { if (virNetServerAddSignalHandler(srv, SIGINT, daemonShutdownHandler, NULL) 0) @@ -1110,6 +1121,8 @@ static int daemonSetupSignals(virNetServerPtr srv) return -1; if (virNetServerAddSignalHandler(srv, SIGTERM, daemonShutdownHandler, NULL) 0) return -1; +if (virNetServerAddSignalHandler(srv, SIGHUP, daemonReloadHandler, NULL) 0) +return -1; return 0; } -- 1.7.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] daemon: Fix regression of libvirtd reloading support
On Mon, Aug 15, 2011 at 15:50:46 +0800, Osier Yang wrote: This is introduced by commit df0b57a95a, which forgot to add signal handler for SIGHUP. A simple reproduce method: 1) Create a domain XML under /etc/libvirt/qemu 2) % kill -SIGHUP $(pidof libvirtd) 3) % virsh list --all (the new created domain XML is not listed) --- daemon/libvirtd.c | 13 + 1 files changed, 13 insertions(+), 0 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index b866a01..3e0159b 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -1102,6 +1102,17 @@ static void daemonShutdownHandler(virNetServerPtr srv, virNetServerQuit(srv); } +static void daemonReloadHandler(virNetServerPtr srv ATTRIBUTE_UNUSED, +siginfo_t *sig ATTRIBUTE_UNUSED, +void *opaque ATTRIBUTE_UNUSED) +{ +VIR_INFO(_(Reloading configuration on SIGHUP)); This message shouldn't be translated. We only translate VIR_ERROR messages. Make syntax-check would fail because of this. +virHookCall(VIR_HOOK_DRIVER_DAEMON, -, +VIR_HOOK_DAEMON_OP_RELOAD, SIGHUP, SIGHUP, NULL); +if (virStateReload() 0) +VIR_WARN(Error while reloading drivers); +} + static int daemonSetupSignals(virNetServerPtr srv) { if (virNetServerAddSignalHandler(srv, SIGINT, daemonShutdownHandler, NULL) 0) @@ -1110,6 +1121,8 @@ static int daemonSetupSignals(virNetServerPtr srv) return -1; if (virNetServerAddSignalHandler(srv, SIGTERM, daemonShutdownHandler, NULL) 0) return -1; +if (virNetServerAddSignalHandler(srv, SIGHUP, daemonReloadHandler, NULL) 0) +return -1; return 0; } ACK with _() removed from VIR_INFO. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] daemon: Fix regression of libvirtd reloading support
于 2011年08月15日 15:39, Jiri Denemark 写道: On Mon, Aug 15, 2011 at 15:50:46 +0800, Osier Yang wrote: This is introduced by commit df0b57a95a, which forgot to add signal handler for SIGHUP. A simple reproduce method: 1) Create a domain XML under /etc/libvirt/qemu 2) % kill -SIGHUP $(pidof libvirtd) 3) % virsh list --all (the new created domain XML is not listed) --- daemon/libvirtd.c | 13 + 1 files changed, 13 insertions(+), 0 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index b866a01..3e0159b 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -1102,6 +1102,17 @@ static void daemonShutdownHandler(virNetServerPtr srv, virNetServerQuit(srv); } +static void daemonReloadHandler(virNetServerPtr srv ATTRIBUTE_UNUSED, +siginfo_t *sig ATTRIBUTE_UNUSED, +void *opaque ATTRIBUTE_UNUSED) +{ +VIR_INFO(_(Reloading configuration on SIGHUP)); This message shouldn't be translated. We only translate VIR_ERROR messages. Make syntax-check would fail because of this. +virHookCall(VIR_HOOK_DRIVER_DAEMON, -, +VIR_HOOK_DAEMON_OP_RELOAD, SIGHUP, SIGHUP, NULL); +if (virStateReload() 0) +VIR_WARN(Error while reloading drivers); +} + static int daemonSetupSignals(virNetServerPtr srv) { if (virNetServerAddSignalHandler(srv, SIGINT, daemonShutdownHandler, NULL) 0) @@ -1110,6 +1121,8 @@ static int daemonSetupSignals(virNetServerPtr srv) return -1; if (virNetServerAddSignalHandler(srv, SIGTERM, daemonShutdownHandler, NULL) 0) return -1; +if (virNetServerAddSignalHandler(srv, SIGHUP, daemonReloadHandler, NULL) 0) +return -1; return 0; } ACK with _() removed from VIR_INFO. Jirka Thanks, I pushed with the change. Osier -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] libvirtd.init.in: stop/restart() - wrong return value in case of fail
On Sat, Aug 13, 2011 at 11:28:42PM -0400, Douglas Schilling Landgraf wrote: Currently the function stop() always return 0 (OK) from killproc() even in case of error. Signed-off-by: Douglas Schilling Landgraf dougsl...@redhat.com --- daemon/libvirtd.init.in |2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/daemon/libvirtd.init.in b/daemon/libvirtd.init.in index aa7870c..0697a2b 100644 --- a/daemon/libvirtd.init.in +++ b/daemon/libvirtd.init.in @@ -76,6 +76,8 @@ stop() { rm -f @localstatedir@/lock/subsys/$SERVICE rm -f $PIDFILE rm -rf @localstatedir@/cache/libvirt/* +else +exit $RETVAL fi } ACK, I also added you to the AUTHORS file with the sending address :-) thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/6] Use fd: protocol for migration
Since qemu doesn't give us any reasonable errors when migration fails because of connection issues, we now create a connection to destination qemu ourselves and just pass the created socket to qemu. Daniel P. Berrange (1): Add API for duplicating a socket/client file descriptor Jiri Denemark (5): Add backlog parameter to virNetSocketListen Support changing UNIX socket owner in virNetSocketNewListenUNIX qemu: Refactor do{Tunnel,Native}Migrate functions qemu: Use virNetSocket for tunneled migration qemu: Use fd: protocol for migration src/qemu/qemu_migration.c | 541 + src/rpc/virnetclient.c| 20 ++ src/rpc/virnetclient.h|3 + src/rpc/virnetserverservice.c |6 +- src/rpc/virnetsocket.c| 29 ++- src/rpc/virnetsocket.h|5 +- tests/virnetsockettest.c | 10 +- 7 files changed, 336 insertions(+), 278 deletions(-) -- 1.7.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/6] qemu: Use virNetSocket for tunneled migration
--- src/qemu/qemu_migration.c | 50 +++- 1 files changed, 8 insertions(+), 42 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 1cabbe0..c29ea9e 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -42,6 +42,7 @@ #include fdstream.h #include uuid.h #include locking/domain_lock.h +#include rpc/virnetsocket.h #define VIR_FROM_THIS VIR_FROM_QEMU @@ -1618,9 +1619,8 @@ static int doTunnelMigrate(struct qemud_driver *driver, unsigned long resource) { qemuDomainObjPrivatePtr priv = vm-privateData; -int qemu_sock = -1; -struct sockaddr_un sa_qemu; char *unixfile = NULL; +virNetSocketPtr sock = NULL; int ret = -1; qemuMigrationSpec spec; @@ -1642,45 +1642,14 @@ static int doTunnelMigrate(struct qemud_driver *driver, goto cleanup; } -qemu_sock = socket(AF_UNIX, SOCK_STREAM, 0); -if (qemu_sock 0) { -virReportSystemError(errno, %s, - _(cannot open tunnelled migration socket)); -goto cleanup; -} -memset(sa_qemu, 0, sizeof(sa_qemu)); -sa_qemu.sun_family = AF_UNIX; -if (virStrcpy(sa_qemu.sun_path, unixfile, - sizeof(sa_qemu.sun_path)) == NULL) { -qemuReportError(VIR_ERR_INTERNAL_ERROR, -_(Unix socket '%s' too big for destination), -unixfile); +if (virNetSocketNewListenUNIX(unixfile, 0700, + driver-user, driver-group, sock) 0 || +virNetSocketListen(sock, 1) 0) goto cleanup; -} -unlink(unixfile); -if (bind(qemu_sock, (struct sockaddr *)sa_qemu, sizeof(sa_qemu)) 0) { -virReportSystemError(errno, - _(Cannot bind to unix socket '%s' for tunnelled migration), - unixfile); -goto cleanup; -} -if (listen(qemu_sock, 1) 0) { -virReportSystemError(errno, - _(Cannot listen on unix socket '%s' for tunnelled migration), - unixfile); -goto cleanup; -} - -if (chown(unixfile, driver-user, driver-group) 0) { -virReportSystemError(errno, - _(Cannot change unix socket '%s' owner), - unixfile); -goto cleanup; -} spec.destType = MIGRATION_DEST_UNIX; spec.dest.unics.file = unixfile; -spec.dest.unics.sock = qemu_sock; +spec.dest.unics.sock = virNetSocketGetFD(sock); spec.fwdType = MIGRATION_FWD_STREAM; spec.fwd.stream = st; @@ -1688,11 +1657,8 @@ static int doTunnelMigrate(struct qemud_driver *driver, cookieoutlen, flags, resource, spec); cleanup: -VIR_FORCE_CLOSE(qemu_sock); -if (unixfile) { -unlink(unixfile); -VIR_FREE(unixfile); -} +virNetSocketFree(sock); +VIR_FREE(unixfile); return ret; } -- 1.7.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/6] Add backlog parameter to virNetSocketListen
So that callers can change the default value. --- src/rpc/virnetserverservice.c |4 ++-- src/rpc/virnetsocket.c|4 ++-- src/rpc/virnetsocket.h|2 +- tests/virnetsockettest.c |6 +++--- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/rpc/virnetserverservice.c b/src/rpc/virnetserverservice.c index 8c9ed1e..e63603f 100644 --- a/src/rpc/virnetserverservice.c +++ b/src/rpc/virnetserverservice.c @@ -127,7 +127,7 @@ virNetServerServicePtr virNetServerServiceNewTCP(const char *nodename, goto error; for (i = 0 ; i svc-nsocks ; i++) { -if (virNetSocketListen(svc-socks[i]) 0) +if (virNetSocketListen(svc-socks[i], 0) 0) goto error; /* IO callback is initially disabled, until we're ready @@ -187,7 +187,7 @@ virNetServerServicePtr virNetServerServiceNewUNIX(const char *path, goto error; for (i = 0 ; i svc-nsocks ; i++) { -if (virNetSocketListen(svc-socks[i]) 0) +if (virNetSocketListen(svc-socks[i], 0) 0) goto error; /* IO callback is initially disabled, until we're ready diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index c222743..c19dcfa 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -1076,10 +1076,10 @@ ssize_t virNetSocketWrite(virNetSocketPtr sock, const char *buf, size_t len) } -int virNetSocketListen(virNetSocketPtr sock) +int virNetSocketListen(virNetSocketPtr sock, int backlog) { virMutexLock(sock-lock); -if (listen(sock-fd, 30) 0) { +if (listen(sock-fd, backlog 0 ? backlog : 30) 0) { virReportSystemError(errno, %s, _(Unable to listen on socket)); virMutexUnlock(sock-lock); return -1; diff --git a/src/rpc/virnetsocket.h b/src/rpc/virnetsocket.h index d6c85d2..24110a6 100644 --- a/src/rpc/virnetsocket.h +++ b/src/rpc/virnetsocket.h @@ -105,7 +105,7 @@ void virNetSocketFree(virNetSocketPtr sock); const char *virNetSocketLocalAddrString(virNetSocketPtr sock); const char *virNetSocketRemoteAddrString(virNetSocketPtr sock); -int virNetSocketListen(virNetSocketPtr sock); +int virNetSocketListen(virNetSocketPtr sock, int backlog); int virNetSocketAccept(virNetSocketPtr sock, virNetSocketPtr *clientsock); diff --git a/tests/virnetsockettest.c b/tests/virnetsockettest.c index e72b9a0..fba7e15 100644 --- a/tests/virnetsockettest.c +++ b/tests/virnetsockettest.c @@ -159,7 +159,7 @@ static int testSocketTCPAccept(const void *opaque) goto cleanup; for (i = 0 ; i nlsock ; i++) { -if (virNetSocketListen(lsock[i]) 0) +if (virNetSocketListen(lsock[i], 0) 0) goto cleanup; } @@ -217,7 +217,7 @@ static int testSocketUNIXAccept(const void *data ATTRIBUTE_UNUSED) if (virNetSocketNewListenUNIX(path, 0700, getgid(), lsock) 0) goto cleanup; -if (virNetSocketListen(lsock) 0) +if (virNetSocketListen(lsock, 0) 0) goto cleanup; if (virNetSocketNewConnectUNIX(path, false, NULL, csock) 0) @@ -276,7 +276,7 @@ static int testSocketUNIXAddrs(const void *data ATTRIBUTE_UNUSED) goto cleanup; } -if (virNetSocketListen(lsock) 0) +if (virNetSocketListen(lsock, 0) 0) goto cleanup; if (virNetSocketNewConnectUNIX(path, false, NULL, csock) 0) -- 1.7.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/6] Support changing UNIX socket owner in virNetSocketNewListenUNIX
This patch allows owner's UID to be changed as well. --- src/rpc/virnetserverservice.c |2 +- src/rpc/virnetsocket.c|7 --- src/rpc/virnetsocket.h|1 + tests/virnetsockettest.c |4 ++-- 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/rpc/virnetserverservice.c b/src/rpc/virnetserverservice.c index e63603f..9f82a8d 100644 --- a/src/rpc/virnetserverservice.c +++ b/src/rpc/virnetserverservice.c @@ -182,7 +182,7 @@ virNetServerServicePtr virNetServerServiceNewUNIX(const char *path, if (virNetSocketNewListenUNIX(path, mask, - grp, + -1, grp, svc-socks[0]) 0) goto error; diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index c19dcfa..23ec5ca 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -302,6 +302,7 @@ error: #if HAVE_SYS_UN_H int virNetSocketNewListenUNIX(const char *path, mode_t mask, + uid_t user, gid_t grp, virNetSocketPtr *retsock) { @@ -344,10 +345,10 @@ int virNetSocketNewListenUNIX(const char *path, /* chown() doesn't work for abstract sockets but we use them only * if libvirtd runs unprivileged */ -if (grp != 0 chown(path, -1, grp)) { +if (grp != 0 chown(path, user, grp)) { virReportSystemError(errno, - _(Failed to change group ID of '%s' to %u), - path, (unsigned int) grp); + _(Failed to change ownership of '%s' to %d:%d), + path, (int) user, (int) grp); goto error; } diff --git a/src/rpc/virnetsocket.h b/src/rpc/virnetsocket.h index 24110a6..f7e5ebb 100644 --- a/src/rpc/virnetsocket.h +++ b/src/rpc/virnetsocket.h @@ -47,6 +47,7 @@ int virNetSocketNewListenTCP(const char *nodename, int virNetSocketNewListenUNIX(const char *path, mode_t mask, + uid_t user, gid_t grp, virNetSocketPtr *addr); diff --git a/tests/virnetsockettest.c b/tests/virnetsockettest.c index fba7e15..fae15a3 100644 --- a/tests/virnetsockettest.c +++ b/tests/virnetsockettest.c @@ -214,7 +214,7 @@ static int testSocketUNIXAccept(const void *data ATTRIBUTE_UNUSED) } } -if (virNetSocketNewListenUNIX(path, 0700, getgid(), lsock) 0) +if (virNetSocketNewListenUNIX(path, 0700, -1, getgid(), lsock) 0) goto cleanup; if (virNetSocketListen(lsock, 0) 0) @@ -263,7 +263,7 @@ static int testSocketUNIXAddrs(const void *data ATTRIBUTE_UNUSED) } } -if (virNetSocketNewListenUNIX(path, 0700, getgid(), lsock) 0) +if (virNetSocketNewListenUNIX(path, 0700, -1, getgid(), lsock) 0) goto cleanup; if (STRNEQ(virNetSocketLocalAddrString(lsock), 127.0.0.1;0)) { -- 1.7.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 6/6] qemu: Use fd: protocol for migration
By opening a connection to remote qemu process ourselves and passing the socket to qemu we get much better errors than just migration failed when the connection is opened by qemu. --- src/qemu/qemu_migration.c | 128 ++--- 1 files changed, 98 insertions(+), 30 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index c29ea9e..537e57e 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1269,6 +1269,7 @@ cleanup: enum qemuMigrationDestinationType { MIGRATION_DEST_HOST, MIGRATION_DEST_UNIX, +MIGRATION_DEST_FD, }; enum qemuMigrationForwardType { @@ -1287,9 +1288,14 @@ struct _qemuMigrationSpec { } host; struct { -const char *file; +char *file; int sock; } unics; /* this sucks but unix is a macro defined to 1 */ + +struct { +int qemu; +int local; +} fd; } dest; enum qemuMigrationForwardType fwdType; @@ -1472,6 +1478,14 @@ qemuMigrationRun(struct qemud_driver *driver, ret = qemuMonitorMigrateToCommand(priv-mon, migrate_flags, args); } break; + +case MIGRATION_DEST_FD: +if (spec-fwdType != MIGRATION_FWD_DIRECT) +fd = spec-dest.fd.local; +ret = qemuMonitorMigrateToFd(priv-mon, migrate_flags, + spec-dest.fd.qemu); +VIR_FORCE_CLOSE(spec-dest.fd.qemu); +break; } qemuDomainObjExitMonitorWithDriver(driver, vm); if (ret 0) @@ -1568,9 +1582,11 @@ static int doNativeMigrate(struct qemud_driver *driver, unsigned int flags, unsigned long resource) { +qemuDomainObjPrivatePtr priv = vm-privateData; xmlURIPtr uribits = NULL; -int ret; +int ret = -1; qemuMigrationSpec spec; +char *tmp = NULL; VIR_DEBUG(driver=%p, vm=%p, uri=%s, cookiein=%s, cookieinlen=%d, cookieout=%p, cookieoutlen=%p, flags=%x, resource=%lu, @@ -1579,13 +1595,12 @@ static int doNativeMigrate(struct qemud_driver *driver, if (STRPREFIX(uri, tcp:) !STRPREFIX(uri, tcp://)) { /* HACK: source host generates bogus URIs, so fix them up */ -char *tmpuri; -if (virAsprintf(tmpuri, tcp://%s, uri + strlen(tcp:)) 0) { +if (virAsprintf(tmp, tcp://%s, uri + strlen(tcp:)) 0) { virReportOOMError(); return -1; } -uribits = xmlParseURI(tmpuri); -VIR_FREE(tmpuri); +uribits = xmlParseURI(tmp); +VIR_FREE(tmp); } else { uribits = xmlParseURI(uri); } @@ -1595,13 +1610,38 @@ static int doNativeMigrate(struct qemud_driver *driver, return -1; } -spec.destType = MIGRATION_DEST_HOST; -spec.dest.host.name = uribits-server; -spec.dest.host.port = uribits-port; spec.fwdType = MIGRATION_FWD_DIRECT; +if (qemuCapsGet(priv-qemuCaps, QEMU_CAPS_MIGRATE_QEMU_FD)) { +virNetSocketPtr sock; + +spec.destType = MIGRATION_DEST_FD; +spec.dest.fd.qemu = -1; + +if (virAsprintf(tmp, %d, uribits-port) 0) { +virReportOOMError(); +goto cleanup; +} +if (virNetSocketNewConnectTCP(uribits-server, tmp, sock) == 0) { +spec.dest.fd.qemu = virNetSocketDupFD(sock, true); +virNetSocketFree(sock); +} +if (spec.dest.fd.qemu == -1) +goto cleanup; +} else { +spec.destType = MIGRATION_DEST_HOST; +spec.dest.host.name = uribits-server; +spec.dest.host.port = uribits-port; +} + ret = qemuMigrationRun(driver, vm, cookiein, cookieinlen, cookieout, cookieoutlen, flags, resource, spec); + +cleanup: +if (spec.destType == MIGRATION_DEST_FD) +VIR_FORCE_CLOSE(spec.dest.fd.qemu); + +VIR_FREE(tmp); xmlFreeURI(uribits); return ret; @@ -1619,7 +1659,6 @@ static int doTunnelMigrate(struct qemud_driver *driver, unsigned long resource) { qemuDomainObjPrivatePtr priv = vm-privateData; -char *unixfile = NULL; virNetSocketPtr sock = NULL; int ret = -1; qemuMigrationSpec spec; @@ -1629,36 +1668,65 @@ static int doTunnelMigrate(struct qemud_driver *driver, driver, vm, st, NULLSTR(cookiein), cookieinlen, cookieout, cookieoutlen, flags, resource); -if (!qemuCapsGet(priv-qemuCaps, QEMU_CAPS_MIGRATE_QEMU_UNIX) +if (!qemuCapsGet(priv-qemuCaps, QEMU_CAPS_MIGRATE_QEMU_FD) +!qemuCapsGet(priv-qemuCaps, QEMU_CAPS_MIGRATE_QEMU_UNIX) !qemuCapsGet(priv-qemuCaps, QEMU_CAPS_MIGRATE_QEMU_EXEC)) { -qemuReportError(VIR_ERR_OPERATION_FAILED, -%s, _(Source qemu is too old to support tunnelled migration)); -goto cleanup; -} - -if (virAsprintf(unixfile,
[libvirt] [PATCH 1/6] Add API for duplicating a socket/client file descriptor
From: Daniel P. Berrange berra...@redhat.com * src/rpc/virnetsocket.c, src/rpc/virnetsocket.h: Add virNetSocketDupFD() * src/rpc/virnetclient.c, src/rpc/virnetclient.h: Add virNetClientDupFD() and virNetClientGetFD() --- src/rpc/virnetclient.c | 20 src/rpc/virnetclient.h |3 +++ src/rpc/virnetsocket.c | 18 ++ src/rpc/virnetsocket.h |2 ++ 4 files changed, 43 insertions(+), 0 deletions(-) diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index b84..31d79ef 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -233,6 +233,26 @@ void virNetClientRef(virNetClientPtr client) } +int virNetClientGetFD(virNetClientPtr client) +{ +int fd; +virNetClientLock(client); +fd = virNetSocketGetFD(client-sock); +virNetClientUnlock(client); +return fd; +} + + +int virNetClientDupFD(virNetClientPtr client, bool cloexec) +{ +int fd; +virNetClientLock(client); +fd = virNetSocketDupFD(client-sock, cloexec); +virNetClientUnlock(client); +return fd; +} + + void virNetClientFree(virNetClientPtr client) { int i; diff --git a/src/rpc/virnetclient.h b/src/rpc/virnetclient.h index 90d19d3..1fabcfd 100644 --- a/src/rpc/virnetclient.h +++ b/src/rpc/virnetclient.h @@ -53,6 +53,9 @@ virNetClientPtr virNetClientNewExternal(const char **cmdargv); void virNetClientRef(virNetClientPtr client); +int virNetClientGetFD(virNetClientPtr client); +int virNetClientDupFD(virNetClientPtr client, bool cloexec); + int virNetClientAddProgram(virNetClientPtr client, virNetClientProgramPtr prog); diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index 992e33a..c222743 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -28,6 +28,7 @@ #include unistd.h #include sys/wait.h #include signal.h +#include fcntl.h #ifdef HAVE_NETINET_TCP_H # include netinet/tcp.h @@ -710,6 +711,23 @@ int virNetSocketGetFD(virNetSocketPtr sock) } +int virNetSocketDupFD(virNetSocketPtr sock, bool cloexec) +{ +int fd; + +if (cloexec) +fd = fcntl(sock-fd, F_DUPFD_CLOEXEC, (long) 0); +else +fd = dup(sock-fd); +if (fd 0) { +virReportSystemError(errno, %s, + _(Unable to copy socket file handle)); +return -1; +} +return fd; +} + + bool virNetSocketIsLocal(virNetSocketPtr sock) { bool isLocal = false; diff --git a/src/rpc/virnetsocket.h b/src/rpc/virnetsocket.h index 1e1c63c..d6c85d2 100644 --- a/src/rpc/virnetsocket.h +++ b/src/rpc/virnetsocket.h @@ -77,6 +77,8 @@ int virNetSocketNewConnectExternal(const char **cmdargv, virNetSocketPtr *addr); int virNetSocketGetFD(virNetSocketPtr sock); +int virNetSocketDupFD(virNetSocketPtr sock, bool cloexec); + bool virNetSocketIsLocal(virNetSocketPtr sock); int virNetSocketGetPort(virNetSocketPtr sock); -- 1.7.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/6] qemu: Refactor do{Tunnel, Native}Migrate functions
The core of these two functions is very similar and most of it is even exactly the same. Factor out the core functionality into a separate function to remove code duplication and make further changes easier. --- src/qemu/qemu_migration.c | 499 ++--- 1 files changed, 239 insertions(+), 260 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 4d0e062..1cabbe0 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1265,122 +1265,37 @@ cleanup: } -/* Perform migration using QEMU's native TCP migrate support, - * not encrypted obviously - */ -static int doNativeMigrate(struct qemud_driver *driver, - virDomainObjPtr vm, - const char *uri, - const char *cookiein, - int cookieinlen, - char **cookieout, - int *cookieoutlen, - unsigned int flags, - const char *dname ATTRIBUTE_UNUSED, - unsigned long resource) -{ -int ret = -1; -xmlURIPtr uribits = NULL; -qemuDomainObjPrivatePtr priv = vm-privateData; -unsigned int background_flags = QEMU_MONITOR_MIGRATE_BACKGROUND; -qemuMigrationCookiePtr mig = NULL; -VIR_DEBUG(driver=%p, vm=%p, uri=%s, cookiein=%s, cookieinlen=%d, - cookieout=%p, cookieoutlen=%p, flags=%x, dname=%s, resource=%lu, - driver, vm, uri, NULLSTR(cookiein), cookieinlen, - cookieout, cookieoutlen, flags, NULLSTR(dname), resource); - -if (virLockManagerPluginUsesState(driver-lockManager) -!cookieout) { -qemuReportError(VIR_ERR_INTERNAL_ERROR, -_(Migration with lock driver %s requires cookie support), -virLockManagerPluginGetName(driver-lockManager)); -return -1; -} - -if (!(mig = qemuMigrationEatCookie(driver, vm, cookiein, cookieinlen, - QEMU_MIGRATION_COOKIE_GRAPHICS))) -goto cleanup; - -if (qemuDomainMigrateGraphicsRelocate(driver, vm, mig) 0) -VIR_WARN(unable to provide data for graphics client relocation); - -/* Issue the migrate command. */ -if (STRPREFIX(uri, tcp:) !STRPREFIX(uri, tcp://)) { -/* HACK: source host generates bogus URIs, so fix them up */ -char *tmpuri; -if (virAsprintf(tmpuri, tcp://%s, uri + strlen(tcp:)) 0) { -virReportOOMError(); -goto cleanup; -} -uribits = xmlParseURI(tmpuri); -VIR_FREE(tmpuri); -} else { -uribits = xmlParseURI(uri); -} -if (!uribits) { -qemuReportError(VIR_ERR_INTERNAL_ERROR, -_(cannot parse URI %s), uri); -goto cleanup; -} - -/* Before EnterMonitor, since qemuProcessStopCPUs already does that */ -if (!(flags VIR_MIGRATE_LIVE) -virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) { -if (qemuMigrationSetOffline(driver, vm) 0) -goto cleanup; -} - -if (qemuDomainObjEnterMonitorAsync(driver, vm, - QEMU_ASYNC_JOB_MIGRATION_OUT) 0) -goto cleanup; - -if (resource 0 -qemuMonitorSetMigrationSpeed(priv-mon, resource) 0) { -qemuDomainObjExitMonitorWithDriver(driver, vm); -goto cleanup; -} - -if (flags VIR_MIGRATE_NON_SHARED_DISK) -background_flags |= QEMU_MONITOR_MIGRATE_NON_SHARED_DISK; - -if (flags VIR_MIGRATE_NON_SHARED_INC) -background_flags |= QEMU_MONITOR_MIGRATE_NON_SHARED_INC; - -if (qemuMonitorMigrateToHost(priv-mon, background_flags, uribits-server, - uribits-port) 0) { -qemuDomainObjExitMonitorWithDriver(driver, vm); -goto cleanup; -} -qemuDomainObjExitMonitorWithDriver(driver, vm); - -if (qemuMigrationWaitForCompletion(driver, vm, - QEMU_ASYNC_JOB_MIGRATION_OUT) 0) -goto cleanup; - -/* When migration completed, QEMU will have paused the - * CPUs for us, but unless we're using the JSON monitor - * we won't have been notified of this, so might still - * think we're running. For v2 protocol this doesn't - * matter because we'll kill the VM soon, but for v3 - * this is important because we stay paused until the - * confirm3 step, but need to release the lock state - */ -if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) { -if (qemuMigrationSetOffline(driver, vm) 0) -goto cleanup; -} - -if (qemuMigrationBakeCookie(mig, driver, vm, cookieout, cookieoutlen, 0) 0) -VIR_WARN(Unable to encode migration cookie); - -ret = 0; +enum qemuMigrationDestinationType { +MIGRATION_DEST_HOST, +MIGRATION_DEST_UNIX, +}; -cleanup: -
Re: [libvirt] [PATCH 1/6] Add API for duplicating a socket/client file descriptor
On Mon, Aug 15, 2011 at 09:58:11AM +0200, Jiri Denemark wrote: From: Daniel P. Berrange berra...@redhat.com * src/rpc/virnetsocket.c, src/rpc/virnetsocket.h: Add virNetSocketDupFD() * src/rpc/virnetclient.c, src/rpc/virnetclient.h: Add virNetClientDupFD() and virNetClientGetFD() --- src/rpc/virnetclient.c | 20 src/rpc/virnetclient.h |3 +++ src/rpc/virnetsocket.c | 18 ++ src/rpc/virnetsocket.h |2 ++ 4 files changed, 43 insertions(+), 0 deletions(-) diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index b84..31d79ef 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -233,6 +233,26 @@ void virNetClientRef(virNetClientPtr client) } +int virNetClientGetFD(virNetClientPtr client) +{ +int fd; +virNetClientLock(client); +fd = virNetSocketGetFD(client-sock); +virNetClientUnlock(client); +return fd; +} + + +int virNetClientDupFD(virNetClientPtr client, bool cloexec) +{ +int fd; +virNetClientLock(client); +fd = virNetSocketDupFD(client-sock, cloexec); +virNetClientUnlock(client); +return fd; +} + + void virNetClientFree(virNetClientPtr client) { int i; diff --git a/src/rpc/virnetclient.h b/src/rpc/virnetclient.h index 90d19d3..1fabcfd 100644 --- a/src/rpc/virnetclient.h +++ b/src/rpc/virnetclient.h @@ -53,6 +53,9 @@ virNetClientPtr virNetClientNewExternal(const char **cmdargv); void virNetClientRef(virNetClientPtr client); +int virNetClientGetFD(virNetClientPtr client); +int virNetClientDupFD(virNetClientPtr client, bool cloexec); + int virNetClientAddProgram(virNetClientPtr client, virNetClientProgramPtr prog); diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index 992e33a..c222743 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -28,6 +28,7 @@ #include unistd.h #include sys/wait.h #include signal.h +#include fcntl.h #ifdef HAVE_NETINET_TCP_H # include netinet/tcp.h @@ -710,6 +711,23 @@ int virNetSocketGetFD(virNetSocketPtr sock) } +int virNetSocketDupFD(virNetSocketPtr sock, bool cloexec) +{ +int fd; + +if (cloexec) +fd = fcntl(sock-fd, F_DUPFD_CLOEXEC, (long) 0); +else +fd = dup(sock-fd); +if (fd 0) { +virReportSystemError(errno, %s, + _(Unable to copy socket file handle)); +return -1; +} +return fd; +} + + bool virNetSocketIsLocal(virNetSocketPtr sock) { bool isLocal = false; diff --git a/src/rpc/virnetsocket.h b/src/rpc/virnetsocket.h index 1e1c63c..d6c85d2 100644 --- a/src/rpc/virnetsocket.h +++ b/src/rpc/virnetsocket.h @@ -77,6 +77,8 @@ int virNetSocketNewConnectExternal(const char **cmdargv, virNetSocketPtr *addr); int virNetSocketGetFD(virNetSocketPtr sock); +int virNetSocketDupFD(virNetSocketPtr sock, bool cloexec); + bool virNetSocketIsLocal(virNetSocketPtr sock); int virNetSocketGetPort(virNetSocketPtr sock); looks fine ACK, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/6] Add backlog parameter to virNetSocketListen
On Mon, Aug 15, 2011 at 09:58:12AM +0200, Jiri Denemark wrote: So that callers can change the default value. --- src/rpc/virnetserverservice.c |4 ++-- src/rpc/virnetsocket.c|4 ++-- src/rpc/virnetsocket.h|2 +- tests/virnetsockettest.c |6 +++--- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/rpc/virnetserverservice.c b/src/rpc/virnetserverservice.c index 8c9ed1e..e63603f 100644 --- a/src/rpc/virnetserverservice.c +++ b/src/rpc/virnetserverservice.c @@ -127,7 +127,7 @@ virNetServerServicePtr virNetServerServiceNewTCP(const char *nodename, goto error; for (i = 0 ; i svc-nsocks ; i++) { -if (virNetSocketListen(svc-socks[i]) 0) +if (virNetSocketListen(svc-socks[i], 0) 0) goto error; /* IO callback is initially disabled, until we're ready @@ -187,7 +187,7 @@ virNetServerServicePtr virNetServerServiceNewUNIX(const char *path, goto error; for (i = 0 ; i svc-nsocks ; i++) { -if (virNetSocketListen(svc-socks[i]) 0) +if (virNetSocketListen(svc-socks[i], 0) 0) goto error; /* IO callback is initially disabled, until we're ready diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index c222743..c19dcfa 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -1076,10 +1076,10 @@ ssize_t virNetSocketWrite(virNetSocketPtr sock, const char *buf, size_t len) } -int virNetSocketListen(virNetSocketPtr sock) +int virNetSocketListen(virNetSocketPtr sock, int backlog) { virMutexLock(sock-lock); -if (listen(sock-fd, 30) 0) { +if (listen(sock-fd, backlog 0 ? backlog : 30) 0) { Okay that's where we override the old default ... virReportSystemError(errno, %s, _(Unable to listen on socket)); virMutexUnlock(sock-lock); return -1; diff --git a/src/rpc/virnetsocket.h b/src/rpc/virnetsocket.h index d6c85d2..24110a6 100644 --- a/src/rpc/virnetsocket.h +++ b/src/rpc/virnetsocket.h @@ -105,7 +105,7 @@ void virNetSocketFree(virNetSocketPtr sock); const char *virNetSocketLocalAddrString(virNetSocketPtr sock); const char *virNetSocketRemoteAddrString(virNetSocketPtr sock); -int virNetSocketListen(virNetSocketPtr sock); +int virNetSocketListen(virNetSocketPtr sock, int backlog); int virNetSocketAccept(virNetSocketPtr sock, virNetSocketPtr *clientsock); diff --git a/tests/virnetsockettest.c b/tests/virnetsockettest.c index e72b9a0..fba7e15 100644 --- a/tests/virnetsockettest.c +++ b/tests/virnetsockettest.c @@ -159,7 +159,7 @@ static int testSocketTCPAccept(const void *opaque) goto cleanup; for (i = 0 ; i nlsock ; i++) { -if (virNetSocketListen(lsock[i]) 0) +if (virNetSocketListen(lsock[i], 0) 0) goto cleanup; } @@ -217,7 +217,7 @@ static int testSocketUNIXAccept(const void *data ATTRIBUTE_UNUSED) if (virNetSocketNewListenUNIX(path, 0700, getgid(), lsock) 0) goto cleanup; -if (virNetSocketListen(lsock) 0) +if (virNetSocketListen(lsock, 0) 0) goto cleanup; if (virNetSocketNewConnectUNIX(path, false, NULL, csock) 0) @@ -276,7 +276,7 @@ static int testSocketUNIXAddrs(const void *data ATTRIBUTE_UNUSED) goto cleanup; } -if (virNetSocketListen(lsock) 0) +if (virNetSocketListen(lsock, 0) 0) goto cleanup; if (virNetSocketNewConnectUNIX(path, false, NULL, csock) 0) Assuming that the facts it compiles prove that all cases got covered, ACK, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/6] Support changing UNIX socket owner in virNetSocketNewListenUNIX
On Mon, Aug 15, 2011 at 09:58:13AM +0200, Jiri Denemark wrote: This patch allows owner's UID to be changed as well. --- src/rpc/virnetserverservice.c |2 +- src/rpc/virnetsocket.c|7 --- src/rpc/virnetsocket.h|1 + tests/virnetsockettest.c |4 ++-- 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/rpc/virnetserverservice.c b/src/rpc/virnetserverservice.c index e63603f..9f82a8d 100644 --- a/src/rpc/virnetserverservice.c +++ b/src/rpc/virnetserverservice.c @@ -182,7 +182,7 @@ virNetServerServicePtr virNetServerServiceNewUNIX(const char *path, if (virNetSocketNewListenUNIX(path, mask, - grp, + -1, grp, Only comment would be that if we started with one line per arg, the patch should probably keep that (but I don't like this much so ...) svc-socks[0]) 0) goto error; diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index c19dcfa..23ec5ca 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -302,6 +302,7 @@ error: #if HAVE_SYS_UN_H int virNetSocketNewListenUNIX(const char *path, mode_t mask, + uid_t user, gid_t grp, virNetSocketPtr *retsock) { @@ -344,10 +345,10 @@ int virNetSocketNewListenUNIX(const char *path, /* chown() doesn't work for abstract sockets but we use them only * if libvirtd runs unprivileged */ -if (grp != 0 chown(path, -1, grp)) { +if (grp != 0 chown(path, user, grp)) { virReportSystemError(errno, - _(Failed to change group ID of '%s' to %u), - path, (unsigned int) grp); + _(Failed to change ownership of '%s' to %d:%d), + path, (int) user, (int) grp); goto error; } diff --git a/src/rpc/virnetsocket.h b/src/rpc/virnetsocket.h index 24110a6..f7e5ebb 100644 --- a/src/rpc/virnetsocket.h +++ b/src/rpc/virnetsocket.h @@ -47,6 +47,7 @@ int virNetSocketNewListenTCP(const char *nodename, int virNetSocketNewListenUNIX(const char *path, mode_t mask, + uid_t user, gid_t grp, virNetSocketPtr *addr); diff --git a/tests/virnetsockettest.c b/tests/virnetsockettest.c index fba7e15..fae15a3 100644 --- a/tests/virnetsockettest.c +++ b/tests/virnetsockettest.c @@ -214,7 +214,7 @@ static int testSocketUNIXAccept(const void *data ATTRIBUTE_UNUSED) } } -if (virNetSocketNewListenUNIX(path, 0700, getgid(), lsock) 0) +if (virNetSocketNewListenUNIX(path, 0700, -1, getgid(), lsock) 0) goto cleanup; if (virNetSocketListen(lsock, 0) 0) @@ -263,7 +263,7 @@ static int testSocketUNIXAddrs(const void *data ATTRIBUTE_UNUSED) } } -if (virNetSocketNewListenUNIX(path, 0700, getgid(), lsock) 0) +if (virNetSocketNewListenUNIX(path, 0700, -1, getgid(), lsock) 0) goto cleanup; if (STRNEQ(virNetSocketLocalAddrString(lsock), 127.0.0.1;0)) { ACK, that too seems uncontroversial Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/6] Add API for duplicating a socket/client file descriptor
On Mon, Aug 15, 2011 at 09:58:11 +0200, Jiri Denemark wrote: From: Daniel P. Berrange berra...@redhat.com * src/rpc/virnetsocket.c, src/rpc/virnetsocket.h: Add virNetSocketDupFD() * src/rpc/virnetclient.c, src/rpc/virnetclient.h: Add virNetClientDupFD() and virNetClientGetFD() --- Ah, I forgot to mention that this patch is in fact a v2 of https://www.redhat.com/archives/libvir-list/2011-July/msg00340.html sent as part of Add a virtlockd lock manager daemon series. I modified it according to Eric's suggestions. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/6] qemu: Refactor do{Tunnel, Native}Migrate functions
On Mon, Aug 15, 2011 at 09:58:14AM +0200, Jiri Denemark wrote: The core of these two functions is very similar and most of it is even exactly the same. Factor out the core functionality into a separate function to remove code duplication and make further changes easier. --- src/qemu/qemu_migration.c | 499 ++--- 1 files changed, 239 insertions(+), 260 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 4d0e062..1cabbe0 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1265,122 +1265,37 @@ cleanup: } -/* Perform migration using QEMU's native TCP migrate support, - * not encrypted obviously - */ -static int doNativeMigrate(struct qemud_driver *driver, - virDomainObjPtr vm, - const char *uri, - const char *cookiein, - int cookieinlen, - char **cookieout, - int *cookieoutlen, - unsigned int flags, - const char *dname ATTRIBUTE_UNUSED, - unsigned long resource) -{ -int ret = -1; -xmlURIPtr uribits = NULL; -qemuDomainObjPrivatePtr priv = vm-privateData; -unsigned int background_flags = QEMU_MONITOR_MIGRATE_BACKGROUND; -qemuMigrationCookiePtr mig = NULL; -VIR_DEBUG(driver=%p, vm=%p, uri=%s, cookiein=%s, cookieinlen=%d, - cookieout=%p, cookieoutlen=%p, flags=%x, dname=%s, resource=%lu, - driver, vm, uri, NULLSTR(cookiein), cookieinlen, - cookieout, cookieoutlen, flags, NULLSTR(dname), resource); - -if (virLockManagerPluginUsesState(driver-lockManager) -!cookieout) { -qemuReportError(VIR_ERR_INTERNAL_ERROR, -_(Migration with lock driver %s requires cookie support), -virLockManagerPluginGetName(driver-lockManager)); -return -1; -} - -if (!(mig = qemuMigrationEatCookie(driver, vm, cookiein, cookieinlen, - QEMU_MIGRATION_COOKIE_GRAPHICS))) -goto cleanup; - -if (qemuDomainMigrateGraphicsRelocate(driver, vm, mig) 0) -VIR_WARN(unable to provide data for graphics client relocation); - -/* Issue the migrate command. */ -if (STRPREFIX(uri, tcp:) !STRPREFIX(uri, tcp://)) { -/* HACK: source host generates bogus URIs, so fix them up */ -char *tmpuri; -if (virAsprintf(tmpuri, tcp://%s, uri + strlen(tcp:)) 0) { -virReportOOMError(); -goto cleanup; -} -uribits = xmlParseURI(tmpuri); -VIR_FREE(tmpuri); -} else { -uribits = xmlParseURI(uri); -} -if (!uribits) { -qemuReportError(VIR_ERR_INTERNAL_ERROR, -_(cannot parse URI %s), uri); -goto cleanup; -} - -/* Before EnterMonitor, since qemuProcessStopCPUs already does that */ -if (!(flags VIR_MIGRATE_LIVE) -virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) { -if (qemuMigrationSetOffline(driver, vm) 0) -goto cleanup; -} - -if (qemuDomainObjEnterMonitorAsync(driver, vm, - QEMU_ASYNC_JOB_MIGRATION_OUT) 0) -goto cleanup; - -if (resource 0 -qemuMonitorSetMigrationSpeed(priv-mon, resource) 0) { -qemuDomainObjExitMonitorWithDriver(driver, vm); -goto cleanup; -} - -if (flags VIR_MIGRATE_NON_SHARED_DISK) -background_flags |= QEMU_MONITOR_MIGRATE_NON_SHARED_DISK; - -if (flags VIR_MIGRATE_NON_SHARED_INC) -background_flags |= QEMU_MONITOR_MIGRATE_NON_SHARED_INC; - -if (qemuMonitorMigrateToHost(priv-mon, background_flags, uribits-server, - uribits-port) 0) { -qemuDomainObjExitMonitorWithDriver(driver, vm); -goto cleanup; -} -qemuDomainObjExitMonitorWithDriver(driver, vm); - -if (qemuMigrationWaitForCompletion(driver, vm, - QEMU_ASYNC_JOB_MIGRATION_OUT) 0) -goto cleanup; - -/* When migration completed, QEMU will have paused the - * CPUs for us, but unless we're using the JSON monitor - * we won't have been notified of this, so might still - * think we're running. For v2 protocol this doesn't - * matter because we'll kill the VM soon, but for v3 - * this is important because we stay paused until the - * confirm3 step, but need to release the lock state - */ -if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) { -if (qemuMigrationSetOffline(driver, vm) 0) -goto cleanup; -} - -if (qemuMigrationBakeCookie(mig, driver, vm, cookieout, cookieoutlen, 0) 0) -
Re: [libvirt] [PATCH 5/6] qemu: Use virNetSocket for tunneled migration
On Mon, Aug 15, 2011 at 09:58:15AM +0200, Jiri Denemark wrote: --- src/qemu/qemu_migration.c | 50 +++- 1 files changed, 8 insertions(+), 42 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 1cabbe0..c29ea9e 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -42,6 +42,7 @@ #include fdstream.h #include uuid.h #include locking/domain_lock.h +#include rpc/virnetsocket.h #define VIR_FROM_THIS VIR_FROM_QEMU @@ -1618,9 +1619,8 @@ static int doTunnelMigrate(struct qemud_driver *driver, unsigned long resource) { qemuDomainObjPrivatePtr priv = vm-privateData; -int qemu_sock = -1; -struct sockaddr_un sa_qemu; char *unixfile = NULL; +virNetSocketPtr sock = NULL; int ret = -1; qemuMigrationSpec spec; @@ -1642,45 +1642,14 @@ static int doTunnelMigrate(struct qemud_driver *driver, goto cleanup; } -qemu_sock = socket(AF_UNIX, SOCK_STREAM, 0); -if (qemu_sock 0) { -virReportSystemError(errno, %s, - _(cannot open tunnelled migration socket)); -goto cleanup; -} -memset(sa_qemu, 0, sizeof(sa_qemu)); -sa_qemu.sun_family = AF_UNIX; -if (virStrcpy(sa_qemu.sun_path, unixfile, - sizeof(sa_qemu.sun_path)) == NULL) { -qemuReportError(VIR_ERR_INTERNAL_ERROR, -_(Unix socket '%s' too big for destination), -unixfile); +if (virNetSocketNewListenUNIX(unixfile, 0700, + driver-user, driver-group, sock) 0 || +virNetSocketListen(sock, 1) 0) If we are sure the errors will be properly reported back in called functions, okay goto cleanup; -} -unlink(unixfile); -if (bind(qemu_sock, (struct sockaddr *)sa_qemu, sizeof(sa_qemu)) 0) { -virReportSystemError(errno, - _(Cannot bind to unix socket '%s' for tunnelled migration), - unixfile); -goto cleanup; -} -if (listen(qemu_sock, 1) 0) { -virReportSystemError(errno, - _(Cannot listen on unix socket '%s' for tunnelled migration), - unixfile); -goto cleanup; -} - -if (chown(unixfile, driver-user, driver-group) 0) { -virReportSystemError(errno, - _(Cannot change unix socket '%s' owner), - unixfile); -goto cleanup; -} spec.destType = MIGRATION_DEST_UNIX; spec.dest.unics.file = unixfile; -spec.dest.unics.sock = qemu_sock; +spec.dest.unics.sock = virNetSocketGetFD(sock); spec.fwdType = MIGRATION_FWD_STREAM; spec.fwd.stream = st; @@ -1688,11 +1657,8 @@ static int doTunnelMigrate(struct qemud_driver *driver, cookieoutlen, flags, resource, spec); cleanup: -VIR_FORCE_CLOSE(qemu_sock); -if (unixfile) { -unlink(unixfile); -VIR_FREE(unixfile); -} +virNetSocketFree(sock); +VIR_FREE(unixfile); return ret; } Looks lovely except that a code removing so many lines on migration code souds too good to be true grin/ ACK, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 6/6] qemu: Use fd: protocol for migration
On Mon, Aug 15, 2011 at 09:58:16AM +0200, Jiri Denemark wrote: By opening a connection to remote qemu process ourselves and passing the socket to qemu we get much better errors than just migration failed when the connection is opened by qemu. --- src/qemu/qemu_migration.c | 128 ++--- 1 files changed, 98 insertions(+), 30 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index c29ea9e..537e57e 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1269,6 +1269,7 @@ cleanup: enum qemuMigrationDestinationType { MIGRATION_DEST_HOST, MIGRATION_DEST_UNIX, +MIGRATION_DEST_FD, }; enum qemuMigrationForwardType { @@ -1287,9 +1288,14 @@ struct _qemuMigrationSpec { } host; struct { -const char *file; +char *file; int sock; } unics; /* this sucks but unix is a macro defined to 1 */ + +struct { +int qemu; +int local; +} fd; } dest; enum qemuMigrationForwardType fwdType; @@ -1472,6 +1478,14 @@ qemuMigrationRun(struct qemud_driver *driver, ret = qemuMonitorMigrateToCommand(priv-mon, migrate_flags, args); } break; + +case MIGRATION_DEST_FD: +if (spec-fwdType != MIGRATION_FWD_DIRECT) +fd = spec-dest.fd.local; +ret = qemuMonitorMigrateToFd(priv-mon, migrate_flags, + spec-dest.fd.qemu); +VIR_FORCE_CLOSE(spec-dest.fd.qemu); Hum, I find dubious that we set up fd variable before calling qemuMonitorMigrateToFd but don't use that variable, smells fishy but I could be wrong since I don't get the full function context, so to double check. +break; } qemuDomainObjExitMonitorWithDriver(driver, vm); if (ret 0) @@ -1568,9 +1582,11 @@ static int doNativeMigrate(struct qemud_driver *driver, unsigned int flags, unsigned long resource) { +qemuDomainObjPrivatePtr priv = vm-privateData; xmlURIPtr uribits = NULL; -int ret; +int ret = -1; qemuMigrationSpec spec; +char *tmp = NULL; VIR_DEBUG(driver=%p, vm=%p, uri=%s, cookiein=%s, cookieinlen=%d, cookieout=%p, cookieoutlen=%p, flags=%x, resource=%lu, @@ -1579,13 +1595,12 @@ static int doNativeMigrate(struct qemud_driver *driver, if (STRPREFIX(uri, tcp:) !STRPREFIX(uri, tcp://)) { /* HACK: source host generates bogus URIs, so fix them up */ -char *tmpuri; -if (virAsprintf(tmpuri, tcp://%s, uri + strlen(tcp:)) 0) { +if (virAsprintf(tmp, tcp://%s, uri + strlen(tcp:)) 0) { virReportOOMError(); return -1; } -uribits = xmlParseURI(tmpuri); -VIR_FREE(tmpuri); +uribits = xmlParseURI(tmp); +VIR_FREE(tmp); } else { uribits = xmlParseURI(uri); } @@ -1595,13 +1610,38 @@ static int doNativeMigrate(struct qemud_driver *driver, return -1; } -spec.destType = MIGRATION_DEST_HOST; -spec.dest.host.name = uribits-server; -spec.dest.host.port = uribits-port; spec.fwdType = MIGRATION_FWD_DIRECT; +if (qemuCapsGet(priv-qemuCaps, QEMU_CAPS_MIGRATE_QEMU_FD)) { +virNetSocketPtr sock; + +spec.destType = MIGRATION_DEST_FD; +spec.dest.fd.qemu = -1; + +if (virAsprintf(tmp, %d, uribits-port) 0) { +virReportOOMError(); +goto cleanup; +} +if (virNetSocketNewConnectTCP(uribits-server, tmp, sock) == 0) { +spec.dest.fd.qemu = virNetSocketDupFD(sock, true); +virNetSocketFree(sock); +} +if (spec.dest.fd.qemu == -1) +goto cleanup; +} else { +spec.destType = MIGRATION_DEST_HOST; +spec.dest.host.name = uribits-server; +spec.dest.host.port = uribits-port; +} + ret = qemuMigrationRun(driver, vm, cookiein, cookieinlen, cookieout, cookieoutlen, flags, resource, spec); + +cleanup: +if (spec.destType == MIGRATION_DEST_FD) +VIR_FORCE_CLOSE(spec.dest.fd.qemu); + +VIR_FREE(tmp); xmlFreeURI(uribits); return ret; @@ -1619,7 +1659,6 @@ static int doTunnelMigrate(struct qemud_driver *driver, unsigned long resource) { qemuDomainObjPrivatePtr priv = vm-privateData; -char *unixfile = NULL; virNetSocketPtr sock = NULL; int ret = -1; qemuMigrationSpec spec; @@ -1629,36 +1668,65 @@ static int doTunnelMigrate(struct qemud_driver *driver, driver, vm, st, NULLSTR(cookiein), cookieinlen, cookieout, cookieoutlen, flags, resource); -if (!qemuCapsGet(priv-qemuCaps,
Re: [libvirt] [PATCH 1/4 v2] pci: Move some pci sriov helper code out of node device driver to util/pci
On 08/12/2011 07:14 PM, Roopa Prabhu wrote: From: Roopa Prabhuropra...@cisco.com This patch moves some of the sriov related pci code from node_device driver to src/util/pci.[ch]. Some functions had to go thru name and argument list change to accommodate the move. Signed-off-by: Roopa Prabhuropra...@cisco.com Signed-off-by: Christian Benvenutibe...@cisco.com Signed-off-by: David Wangdwa...@cisco.com --- src/Makefile.am |5 + src/conf/node_device_conf.c |1 src/conf/node_device_conf.h |7 - src/node_device/node_device_driver.h | 14 -- src/node_device/node_device_hal.c | 10 + src/node_device/node_device_linux_sysfs.c | 191 src/node_device/node_device_udev.c| 10 + src/util/pci.c| 196 + src/util/pci.h| 25 9 files changed, 242 insertions(+), 217 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index 009ff25..4246823 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -953,7 +953,10 @@ libvirt_driver_nodedev_la_SOURCES = $(NODE_DEVICE_DRIVER_SOURCES) libvirt_driver_nodedev_la_CFLAGS = \ -I@top_srcdir@/src/conf $(AM_CFLAGS) libvirt_driver_nodedev_la_LDFLAGS = $(AM_LDFLAGS) -libvirt_driver_nodedev_la_LIBADD = +libvirt_driver_nodedev_la_LIBADD = \ + libvirt_util.la \ + ../gnulib/lib/libgnu.la + if HAVE_HAL libvirt_driver_nodedev_la_SOURCES += $(NODE_DEVICE_DRIVER_HAL_SOURCES) libvirt_driver_nodedev_la_CFLAGS += $(HAL_CFLAGS) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index dde2921..548bbff 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -36,6 +36,7 @@ #include util.h #include buf.h #include uuid.h +#include pci.h #define VIR_FROM_THIS VIR_FROM_NODEDEV diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index cef86d4..17be031 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -82,13 +82,6 @@ enum virNodeDevPCICapFlags { VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION= (1 1), }; -struct pci_config_address { -unsigned int domain; -unsigned int bus; -unsigned int slot; -unsigned int function; -}; - typedef struct _virNodeDevCapsDef virNodeDevCapsDef; typedef virNodeDevCapsDef *virNodeDevCapsDefPtr; struct _virNodeDevCapsDef { diff --git a/src/node_device/node_device_driver.h b/src/node_device/node_device_driver.h index 08779b1..673e95b 100644 --- a/src/node_device/node_device_driver.h +++ b/src/node_device/node_device_driver.h @@ -37,10 +37,6 @@ # define LINUX_SYSFS_VPORT_CREATE_POSTFIX /vport_create # define LINUX_SYSFS_VPORT_DELETE_POSTFIX /vport_delete -# define SRIOV_FOUND 0 -# define SRIOV_NOT_FOUND 1 -# define SRIOV_ERROR -1 - # define LINUX_NEW_DEVICE_WAIT_TIME 60 # ifdef HAVE_HAL @@ -63,14 +59,6 @@ int check_fc_host_linux(union _virNodeDevCapData *d); # define check_vport_capable(d) check_vport_capable_linux(d) int check_vport_capable_linux(union _virNodeDevCapData *d); -# define get_physical_function(s,d) get_physical_function_linux(s,d) -int get_physical_function_linux(const char *sysfs_path, -union _virNodeDevCapData *d); - -# define get_virtual_functions(s,d) get_virtual_functions_linux(s,d) -int get_virtual_functions_linux(const char *sysfs_path, -union _virNodeDevCapData *d); - # define read_wwn(host, file, wwn) read_wwn_linux(host, file, wwn) int read_wwn_linux(int host, const char *file, char **wwn); @@ -78,8 +66,6 @@ int read_wwn_linux(int host, const char *file, char **wwn); # define check_fc_host(d) (-1) # define check_vport_capable(d)(-1) -# define get_physical_function(sysfs_path, d) -# define get_virtual_functions(sysfs_path, d) # define read_wwn(host, file, wwn) # endif /* __linux__ */ diff --git a/src/node_device/node_device_hal.c b/src/node_device/node_device_hal.c index 421f5ad..481be97 100644 --- a/src/node_device/node_device_hal.c +++ b/src/node_device/node_device_hal.c @@ -35,6 +35,7 @@ #include datatypes.h #include memory.h #include uuid.h +#include pci.h #include logging.h #include node_device_driver.h @@ -146,8 +147,13 @@ static int gather_pci_cap(LibHalContext *ctx, const char *udi, (void)virStrToLong_ui(p+1,p, 16,d-pci_dev.function); } -get_physical_function(sysfs_path, d); -get_virtual_functions(sysfs_path, d); +if (!pciGetPhysicalFunction(sysfs_path,d-pci_dev.physical_function)) +d-pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION; + +if (!pciGetVirtualFunctions(sysfs_path,d-pci_dev.virtual_functions, +d-pci_dev.num_virtual_functions) || +d-pci_dev.num_virtual_functions 0)
Re: [libvirt] [PATCH 1/4 v2] pci: Move some pci sriov helper code out of node device driver to util/pci
On 08/12/2011 07:14 PM, Roopa Prabhu wrote: -get_physical_function(sysfs_path, d); -get_virtual_functions(sysfs_path, d); +if (!pciGetPhysicalFunction(sysfs_path,d-pci_dev.physical_function)) +d-pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION; + +if (!pciGetVirtualFunctions(sysfs_path,d-pci_dev.virtual_functions, +d-pci_dev.num_virtual_functions) || +d-pci_dev.num_virtual_functions 0) +d-pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION; VIR_FREE(sysfs_path); [...] diff --git a/src/util/pci.h b/src/util/pci.h index a351baf..367881e 100644 --- a/src/util/pci.h +++ b/src/util/pci.h @@ -27,6 +27,13 @@ typedef struct _pciDevice pciDevice; typedef struct _pciDeviceList pciDeviceList; +struct pci_config_address { +unsigned int domain; +unsigned int bus; +unsigned int slot; +unsigned int function; +}; + pciDevice *pciGetDevice (unsigned domain, unsigned bus, unsigned slot, @@ -74,4 +81,22 @@ int pciDeviceIsAssignable(pciDevice *dev, int strict_acs_check); int pciWaitForDeviceCleanup(pciDevice *dev, const char *matcher); +#ifdef __linux__ + +# define pciGetPhysicalFunction(s,a) pciGetPhysicalFunctionLinux(s,a) +int pciGetPhysicalFunctionLinux(const char *sysfs_path, +struct pci_config_address **phys_fn); + +# define pciGetVirtualFunctions(s,a,n) pciGetVirtualFunctionsLinux(s,a,n) +int pciGetVirtualFunctionsLinux(const char *sysfs_path, +struct pci_config_address ***virtual_functions, +unsigned int *num_virtual_functions); + +#else /* __linux__ */ + +# define pciGetPhysicalFunction(s,a) +# define pciGetVirtualFunctions(s,a,n) + I don't think these #defines will produce compilable code if they are used above. You'll probably have to implement functions for them. Stefan +#endif + #endif /* __VIR_PCI_H__ */ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/4 v2] pci: Add helper functions for sriov devices
On 08/12/2011 07:14 PM, Roopa Prabhu wrote: From: Roopa Prabhuropra...@cisco.com This patch adds the following helper functions: pciDeviceIsVirtualFunction: Function to check if a pci device is a sriov VF pciGetVirtualFunctionIndex: Function to get the VF index of a sriov VF pciDeviceNetName: Function to get the network device name of a pci device pciConfigAddressCompare: Function to compare pci config addresses Signed-off-by: Roopa Prabhuropra...@cisco.com Signed-off-by: Christian Benvenutibe...@cisco.com Signed-off-by: David Wangdwa...@cisco.com --- src/util/pci.c | 118 src/util/pci.h | 15 +++ 2 files changed, 133 insertions(+), 0 deletions(-) diff --git a/src/util/pci.c b/src/util/pci.c index e017db9..558c7ae 100644 --- a/src/util/pci.c +++ b/src/util/pci.c @@ -1875,3 +1875,121 @@ out: return ret; } + +/* + * returns 0 if equal and 1 if not + */ +static int +pciConfigAddressCompare(struct pci_config_address *bdf1, +struct pci_config_address *bdf2) +{ +return !((bdf1-domain == bdf2-domain) + (bdf1-bus == bdf2-bus) + (bdf1-slot == bdf2-slot) + (bdf1-function == bdf2-function)); +} Would it no be more intuitive to call implement a function pciConfigAddressEqual and return '1' in case the addresses are equal? + +/* + * Returns 1 if vf device is a virtual function, 0 if not, -1 on error + */ +int +pciDeviceIsVirtualFunctionLinux(const char *vf_sysfs_device_link) +{ +char *vf_sysfs_physfn_link = NULL; +int ret = -1; + +if (virAsprintf(vf_sysfs_physfn_link, %s/physfn, +vf_sysfs_device_link) 0) { +virReportOOMError(); +return ret; +} + +ret = virFileExists(vf_sysfs_physfn_link); + +VIR_FREE(vf_sysfs_physfn_link); + +return ret; +} + +/* + * Returns the sriov virtual function index of vf given its pf + */ +int +pciGetVirtualFunctionIndexLinux(const char *pf_sysfs_device_link, +const char *vf_sysfs_device_link, +int *vf_index) +{ +int ret = -1, i; +unsigned int num_virt_fns = 0; +struct pci_config_address *vf_bdf = NULL; +struct pci_config_address **virt_fns = NULL; + +if (pciGetPciConfigAddressFromSysfsDeviceLink(vf_sysfs_device_link, +vf_bdf)) +return ret; + +if (pciGetVirtualFunctionsLinux(pf_sysfs_device_link,virt_fns, +num_virt_fns)) { +VIR_DEBUG(Error getting physical function's '%s' virtual_functions\n, + pf_sysfs_device_link); +goto out; +} + +for (i = 0; i num_virt_fns; i++) { + if (!pciConfigAddressCompare(vf_bdf, virt_fns[i])) { + *vf_index = i; + ret = 0; + break; + } +} + +out: + +/* free virtual functions */ +for (i = 0; i num_virt_fns; i++) + VIR_FREE(virt_fns[i]); + +VIR_FREE(vf_bdf); + +return ret; +} + +/* + * Returns the network device name of a pci device + */ +int +pciDeviceNetNameLinux(char *device_link_sysfs_path, char **netname) +{ + char *pcidev_sysfs_net_path = NULL; + int ret = -1; + DIR *dir = NULL; + struct dirent *entry = NULL; + + if (virBuildPath(pcidev_sysfs_net_path, device_link_sysfs_path, + net) == -1) { + virReportOOMError(); + return -1; + } + + dir = opendir(pcidev_sysfs_net_path); + if (dir == NULL) + goto out; + + while ((entry = readdir(dir))) { +if (!strcmp(entry-d_name, .) || +!strcmp(entry-d_name, ..)) +continue; + +/* Assume a single directory entry */ +*netname = strdup(entry-d_name); +ret = 0; +break; + } + + closedir(dir); + +out: + VIR_FREE(pcidev_sysfs_net_path); + + return ret; +} diff --git a/src/util/pci.h b/src/util/pci.h index 367881e..0cdc6ec 100644 --- a/src/util/pci.h +++ b/src/util/pci.h @@ -92,10 +92,25 @@ int pciGetVirtualFunctionsLinux(const char *sysfs_path, struct pci_config_address ***virtual_functions, unsigned int *num_virtual_functions); +# define pciDeviceIsVirtualFunction(v) pciDeviceIsVirtualFunctionLinux(v) +int pciDeviceIsVirtualFunctionLinux(const char *vf_sysfs_device_link); + +# define pciGetVirtualFunctionIndex(p,v,i) \ + pciGetVirtualFunctionIndexLinux(p,v,i) +int pciGetVirtualFunctionIndexLinux(const char *pf_sysfs_device_link, +const char *vf_sysfs_device_link, +int *vf_index); + +# define pciDeviceNetName(d,n) pciDeviceNetNameLinux(d,n) +int pciDeviceNetNameLinux(char *device_link_sysfs_path, char **netname); + #else /* __linux__ */ # define pciGetPhysicalFunction(s,a) # define pciGetVirtualFunctions(s,a,n) +# define pciDeviceIsVirtualFunction(v) Also here is there is code like 'if
Re: [libvirt] [PATCH 4/4 v2] macvtap: Fix getPhysfn to get the PF of a direct attach network interface
On 08/12/2011 07:14 PM, Roopa Prabhu wrote: From: Roopa Prabhuropra...@cisco.com This patch renames getPhysfn to getPhysfnDev and adds code to get the Physical function and Virtual Function index of the direct attach linkdev (if the direct attach interface is a SRIOV VF). The idea is to send the port profile message to a PF if the direct attach interface is a SRIOV VF. Signed-off-by: Roopa Prabhuropra...@cisco.com Signed-off-by: Christian Benvenutibe...@cisco.com Signed-off-by: David Wangdwa...@cisco.com --- src/util/macvtap.c | 23 ++- 1 files changed, 10 insertions(+), 13 deletions(-) diff --git a/src/util/macvtap.c b/src/util/macvtap.c index 0b00776..9bf7fa6 100644 --- a/src/util/macvtap.c +++ b/src/util/macvtap.c @@ -943,23 +943,20 @@ err_exit: # ifdef IFLA_VF_PORT_MAX static int -getPhysfn(const char *linkdev, - int32_t *vf, - char **physfndev) +getPhysfnDev(const char *linkdev, + int32_t *vf, + char **physfndev) { int rc = 0; -bool virtfn = false; -if (virtfn) { +if (ifaceIsVirtualFunction(linkdev)) { -/* XXX: if linkdev is SR-IOV VF, then set vf = VF index */ -/* XXX: and set linkdev = PF device */ -/* XXX: need to use get_physical_function_linux() or */ -/* XXX: something like that to get PF */ -/* XXX: device and figure out VF index */ - -rc = 1; +/* if linkdev is SR-IOV VF, then set vf = VF index */ +/* and set linkdev = PF device */ +rc = ifaceGetPhysicalFunction(linkdev, physfndev); +if (!rc) +rc = ifaceGetVirtualFunctionIndex(*physfndev, linkdev, vf); } else { /* Not SR-IOV VF: physfndev is linkdev and VF index @@ -1003,7 +1000,7 @@ doPortProfileOp8021Qbh(const char *ifname, int ifindex; int vlanid = -1; -rc = getPhysfn(ifname,vf,physfndev); +rc = getPhysfnDev(ifname,vf,physfndev); if (rc) goto err_exit; ACK. Stefan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/4 v2] interface: Add functions to get sriov PF/VF relationship of a net interface
On 08/12/2011 07:14 PM, Roopa Prabhu wrote: From: Roopa Prabhuropra...@cisco.com This patch adds the following functions to get PF/VF relationship of an SRIOV network interface: ifaceIsVirtualFunction: Function to check if a network interface is a SRIOV VF ifaceGetVirtualFunctionIndex: Function to get VF index if a network interface is a SRIOV VF ifaceGetPhysicalFunction: Function to get the PF net interface name of a SRIOV VF net interface Signed-off-by: Roopa Prabhuropra...@cisco.com Signed-off-by: Christian Benvenutibe...@cisco.com Signed-off-by: David Wangdwa...@cisco.com --- src/util/interface.c | 150 ++ src/util/interface.h |9 +++ 2 files changed, 159 insertions(+), 0 deletions(-) diff --git a/src/util/interface.c b/src/util/interface.c index 8b4522b..ec00606 100644 --- a/src/util/interface.c +++ b/src/util/interface.c @@ -45,6 +45,8 @@ #include virfile.h #include memory.h #include netlink.h +#include pci.h +#include logging.h #define VIR_FROM_THIS VIR_FROM_NET @@ -1196,3 +1198,151 @@ ifaceRestoreMacAddress(const char *linkdev, return rc; } + +#if __linux__ +static int +ifaceSysfsFile(char **pf_sysfs_device_link, const char *ifname, + const char *file) +{ + +if (virAsprintf(pf_sysfs_device_link, NET_SYSFS %s/%s, +ifname, file) 0) { +virReportOOMError(); +return -1; +} + +return 0; +} + +static int +ifaceSysfsDeviceFile(char **pf_sysfs_device_link, const char *ifname, + const char *file) +{ + +if (virAsprintf(pf_sysfs_device_link, NET_SYSFS %s/device/%s, +ifname, file) 0) { +virReportOOMError(); +return -1; +} + +return 0; +} + +/** + * ifaceIsVirtualFunction + * + * @ifname : name of the interface + * + * Checks if an interface is a SRIOV virtual function. + * + * Returns 1 if interface is SRIOV virtual function, 0 if not and -1 if error + * + */ +int +ifaceIsVirtualFunction(const char *ifname) +{ +char *if_sysfs_device_link = NULL; +int ret = -1; + +if (ifaceSysfsFile(if_sysfs_device_link, ifname, device)) +return ret; + +ret = pciDeviceIsVirtualFunction(if_sysfs_device_link); + +VIR_FREE(if_sysfs_device_link); + +return ret; +} + +/** + * ifaceGetVirtualFunctionIndex + * + * @pfname : name of the physical function interface name + * @vfname : name of the virtual function interface name + * @vf_index : Pointer to int. Contains vf index of interface upon successful + * return + * + * Returns 0 on success, -1 on failure + * + */ +int +ifaceGetVirtualFunctionIndex(const char *pfname, const char *vfname, + int *vf_index) +{ +char *pf_sysfs_device_link = NULL, *vf_sysfs_device_link = NULL; +int ret = -1; + +if (ifaceSysfsFile(pf_sysfs_device_link, pfname, device)) +return ret; + +if (ifaceSysfsFile(vf_sysfs_device_link, vfname, device)) { +VIR_FREE(pf_sysfs_device_link); +return ret; +} + +ret = pciGetVirtualFunctionIndex(pf_sysfs_device_link, + vf_sysfs_device_link, + vf_index); + +VIR_FREE(pf_sysfs_device_link); +VIR_FREE(vf_sysfs_device_link); + +return ret; +} + +/** + * ifaceGetPhysicalFunction + * + * @ifname : name of the physical function interface name + * @pfname : Contains sriov physical function for interface ifname + * upon successful return + * + * Returns 0 on success, -1 on failure + * + */ +int +ifaceGetPhysicalFunction(const char *ifname, char **pfname) +{ +char *physfn_sysfs_path = NULL; +int ret = -1; + +if (ifaceSysfsDeviceFile(physfn_sysfs_path, ifname, physfn)) +return ret; + +ret = pciDeviceNetName(physfn_sysfs_path, pfname); + +VIR_FREE(physfn_sysfs_path); + +return ret; +} +#else + +int +ifaceIsVirtualFunction(const char *ifname) +{ +ifaceError(VIR_ERR_INTERNAL_ERROR, %s, + _(ifaceIsVirtualFunction is not supported on non-linux + platforms)); +return -ENOSYS; Since above functions are described to return -1 on error, also do this in these ones here. Stefan +} + +int +ifaceGetVirtualFunctionIndex(const char *pfname, const char *vfname, + int *vf_index) +{ +ifaceError(VIR_ERR_INTERNAL_ERROR, %s, + _(ifaceGetVirtualFunctionIndex is not supported on non-linux + platforms)); +return -ENOSYS; +} + +int +ifaceGetPhysicalFunction(const char *ifname, char **pfname) +{ +ifaceError(VIR_ERR_INTERNAL_ERROR, %s, + _(ifaceGetPhysicalFunction is not supported on non-linux + platforms)); +return -ENOSYS; +} + +#endif /* __linux__ */ diff --git a/src/util/interface.h b/src/util/interface.h index 47c0eb0..bdd37e5 100644 --- a/src/util/interface.h +++ b/src/util/interface.h @@ -27,6 +27,8 @@ struct nlattr; # include
Re: [libvirt] [PATCH 3/6] Support changing UNIX socket owner in virNetSocketNewListenUNIX
On Mon, Aug 15, 2011 at 17:38:47 +0800, Daniel Veillard wrote: On Mon, Aug 15, 2011 at 09:58:13AM +0200, Jiri Denemark wrote: This patch allows owner's UID to be changed as well. --- src/rpc/virnetserverservice.c |2 +- src/rpc/virnetsocket.c|7 --- src/rpc/virnetsocket.h|1 + tests/virnetsockettest.c |4 ++-- 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/rpc/virnetserverservice.c b/src/rpc/virnetserverservice.c index e63603f..9f82a8d 100644 --- a/src/rpc/virnetserverservice.c +++ b/src/rpc/virnetserverservice.c @@ -182,7 +182,7 @@ virNetServerServicePtr virNetServerServiceNewUNIX(const char *path, if (virNetSocketNewListenUNIX(path, mask, - grp, + -1, grp, Only comment would be that if we started with one line per arg, the patch should probably keep that (but I don't like this much so ...) Yeah, I was thinking about uid and gid to be so closely related that it made sense to put them on a single line. I separated them for consistency. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] RFC (V2) New virDomainBlockPull API family to libvirt
On 08/14/2011 11:40 PM, Zhi Yong Wu wrote: HI, Deniel and Adam. Have the patchset been merged into libvirt upstream? Yes they have. However, the functionality is still missing from qemu. The two communities have agreed upon the interface and semantics, but work continues on the qemu implementation. Let me know if you would like a link to some qemu patches that support this functionality for qed images. -- Adam Litke IBM Linux Technology Center -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/6] qemu: Refactor do{Tunnel, Native}Migrate functions
On Mon, Aug 15, 2011 at 17:48:03 +0800, Daniel Veillard wrote: On Mon, Aug 15, 2011 at 09:58:14AM +0200, Jiri Denemark wrote: The core of these two functions is very similar and most of it is even exactly the same. Factor out the core functionality into a separate function to remove code duplication and make further changes easier. --- src/qemu/qemu_migration.c | 499 ++--- 1 files changed, 239 insertions(+), 260 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 4d0e062..1cabbe0 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1265,122 +1265,37 @@ cleanup: ... +struct _qemuMigrationSpec { +enum qemuMigrationDestinationType destType; +union { +struct { +const char *name; +int port; +} host; + +struct { +const char *file; +int sock; +} unics; /* this sucks but unix is a macro defined to 1 */ stylistic: I would go for unx, unics is so reminiscent of Multics sigh/ or unix_socket, that one should be clean. OK, I kind of like this link to Multics :-) but I agree that unix_socket is cleaner (though unix_socket.sock is not so nice as unix.sock would have been) and I also removed the comment. ... +qemuMigrationRun(struct qemud_driver *driver, + virDomainObjPtr vm, + const char *cookiein, + int cookieinlen, + char **cookieout, + int *cookieoutlen, + unsigned int flags, + unsigned long resource, + qemuMigrationSpecPtr spec) { -qemuDomainObjPrivatePtr priv = vm-privateData; -int client_sock = -1; -int qemu_sock = -1; -struct sockaddr_un sa_qemu, sa_client; -socklen_t addrlen; -int status; -unsigned long long transferred, remaining, total; -char *unixfile = NULL; -unsigned int background_flags = QEMU_MONITOR_MIGRATE_BACKGROUND; int ret = -1; +unsigned int migrate_flags = QEMU_MONITOR_MIGRATE_BACKGROUND; +qemuDomainObjPrivatePtr priv = vm-privateData; qemuMigrationCookiePtr mig = NULL; qemuMigrationIOThreadPtr iothread = NULL; -VIR_DEBUG(driver=%p, vm=%p, st=%p, cookiein=%s, cookieinlen=%d, - cookieout=%p, cookieoutlen=%p, flags=%lx, resource=%lu, - driver, vm, st, NULLSTR(cookiein), cookieinlen, - cookieout, cookieoutlen, flags, resource); I would still keep a VIR_DEBUG() even if there is one on the callers, because that code is new, not that simple, and I would go for increased tracability/verbosity in that case OK, I added this additional VIR_DEBUG to qemuMigrationRun too. Note that the existing do{Native,Tunnel}Migrate didn't lose their VIR_DEBUG calls. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 5/6] qemu: Use virNetSocket for tunneled migration
On Mon, Aug 15, 2011 at 17:55:33 +0800, Daniel Veillard wrote: On Mon, Aug 15, 2011 at 09:58:15AM +0200, Jiri Denemark wrote: --- src/qemu/qemu_migration.c | 50 +++- 1 files changed, 8 insertions(+), 42 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 1cabbe0..c29ea9e 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -42,6 +42,7 @@ ... -qemu_sock = socket(AF_UNIX, SOCK_STREAM, 0); -if (qemu_sock 0) { -virReportSystemError(errno, %s, - _(cannot open tunnelled migration socket)); -goto cleanup; -} -memset(sa_qemu, 0, sizeof(sa_qemu)); -sa_qemu.sun_family = AF_UNIX; -if (virStrcpy(sa_qemu.sun_path, unixfile, - sizeof(sa_qemu.sun_path)) == NULL) { -qemuReportError(VIR_ERR_INTERNAL_ERROR, -_(Unix socket '%s' too big for destination), -unixfile); +if (virNetSocketNewListenUNIX(unixfile, 0700, + driver-user, driver-group, sock) 0 || +virNetSocketListen(sock, 1) 0) If we are sure the errors will be properly reported back in called functions, okay Yep, both virNetSocketNewListenUNIX and virNetSocketListen report the errors themselves. Looks lovely except that a code removing so many lines on migration code souds too good to be true grin/ :-P Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/6] Add API for duplicating a socket/client file descriptor
On 08/15/2011 01:58 AM, Jiri Denemark wrote: @@ -710,6 +711,23 @@ int virNetSocketGetFD(virNetSocketPtr sock) } +int virNetSocketDupFD(virNetSocketPtr sock, bool cloexec) +{ +int fd; + +if (cloexec) +fd = fcntl(sock-fd, F_DUPFD_CLOEXEC, (long) 0); The third argument is unneeded, and skipping it will avoid a stupid-looking cast. But leaving it doesn't hurt either, if you already pushed. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 6/6] qemu: Use fd: protocol for migration
On Mon, Aug 15, 2011 at 18:04:07 +0800, Daniel Veillard wrote: On Mon, Aug 15, 2011 at 09:58:16AM +0200, Jiri Denemark wrote: By opening a connection to remote qemu process ourselves and passing the socket to qemu we get much better errors than just migration failed when the connection is opened by qemu. --- src/qemu/qemu_migration.c | 128 ++--- 1 files changed, 98 insertions(+), 30 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index c29ea9e..537e57e 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1269,6 +1269,7 @@ cleanup: ... enum qemuMigrationDestinationType { MIGRATION_DEST_HOST, MIGRATION_DEST_UNIX, +MIGRATION_DEST_FD, }; enum qemuMigrationForwardType { @@ -1287,9 +1288,14 @@ struct _qemuMigrationSpec { } host; struct { -const char *file; +char *file; int sock; } unics; /* this sucks but unix is a macro defined to 1 */ + +struct { +int qemu; +int local; +} fd; } dest; enum qemuMigrationForwardType fwdType; @@ -1472,6 +1478,14 @@ qemuMigrationRun(struct qemud_driver *driver, ret = qemuMonitorMigrateToCommand(priv-mon, migrate_flags, args); } break; + +case MIGRATION_DEST_FD: +if (spec-fwdType != MIGRATION_FWD_DIRECT) +fd = spec-dest.fd.local; +ret = qemuMonitorMigrateToFd(priv-mon, migrate_flags, + spec-dest.fd.qemu); +VIR_FORCE_CLOSE(spec-dest.fd.qemu); Hum, I find dubious that we set up fd variable before calling qemuMonitorMigrateToFd but don't use that variable, smells fishy but I could be wrong since I don't get the full function context, so to double check. The fd variable contains local end of a pipe or socket, that is it's the file descriptor we will later read migration data from and forward through virStream. Okay, I guess the best is to apply and run the verious test suites on it ! Out of curiosity did you ran dan's migration suite with those patches ? Unfortunately, I still don't have the setup for doing that. However, I did test all kinds (normal, p2p, tunneled) of migration after the refactoring patch, after switching unix socket creation to virNetSocket, and after this patch and all tests (after fixing some bugs and hitting some other unrelated bugs that will be fixed later) went fine. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/6] Add API for duplicating a socket/client file descriptor
On Mon, Aug 15, 2011 at 06:42:15 -0600, Eric Blake wrote: On 08/15/2011 01:58 AM, Jiri Denemark wrote: @@ -710,6 +711,23 @@ int virNetSocketGetFD(virNetSocketPtr sock) } +int virNetSocketDupFD(virNetSocketPtr sock, bool cloexec) +{ +int fd; + +if (cloexec) +fd = fcntl(sock-fd, F_DUPFD_CLOEXEC, (long) 0); The third argument is unneeded, and skipping it will avoid a stupid-looking cast. But leaving it doesn't hurt either, if you already pushed. Ah, nice, fcntl man page didn't mention this possibility. I was just about to push but I'll remove the argument first. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/6] Add API for duplicating a socket/client file descriptor
On 08/15/2011 03:43 AM, Jiri Denemark wrote: On Mon, Aug 15, 2011 at 09:58:11 +0200, Jiri Denemark wrote: From: Daniel P. Berrangeberra...@redhat.com * src/rpc/virnetsocket.c, src/rpc/virnetsocket.h: Add virNetSocketDupFD() * src/rpc/virnetclient.c, src/rpc/virnetclient.h: Add virNetClientDupFD() and virNetClientGetFD() --- Ah, I forgot to mention that this patch is in fact a v2 of https://www.redhat.com/archives/libvir-list/2011-July/msg00340.html sent as part of Add a virtlockd lock manager daemon series. I modified it according to Eric's suggestions. Except you missed one change I had pointed out there: bootstrap.conf needs to list 'fcntl' in the list of gnulib modules (without it, gnulib doesn't guarantee that F_DUPFD_CLOEXEC will work on mingw). -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/6] Add API for duplicating a socket/client file descriptor
On Mon, Aug 15, 2011 at 06:49:58 -0600, Eric Blake wrote: On 08/15/2011 03:43 AM, Jiri Denemark wrote: On Mon, Aug 15, 2011 at 09:58:11 +0200, Jiri Denemark wrote: From: Daniel P. Berrangeberra...@redhat.com * src/rpc/virnetsocket.c, src/rpc/virnetsocket.h: Add virNetSocketDupFD() * src/rpc/virnetclient.c, src/rpc/virnetclient.h: Add virNetClientDupFD() and virNetClientGetFD() --- Ah, I forgot to mention that this patch is in fact a v2 of https://www.redhat.com/archives/libvir-list/2011-July/msg00340.html sent as part of Add a virtlockd lock manager daemon series. I modified it according to Eric's suggestions. Except you missed one change I had pointed out there: bootstrap.conf needs to list 'fcntl' in the list of gnulib modules (without it, gnulib doesn't guarantee that F_DUPFD_CLOEXEC will work on mingw). Hmm, I saw fcntl-h in bootstrap.conf, is that something else? Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: error message should show uri instead of (null)
Fix pointer for error message uri if domain migration fails. BZ# 730244 --- src/qemu/qemu_migration.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 4d0e062..d900020 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2079,7 +2079,7 @@ static int doPeer2PeerMigrate(struct qemud_driver *driver, qemuDomainObjExitRemoteWithDriver(driver, vm); if (dconn == NULL) { qemuReportError(VIR_ERR_OPERATION_FAILED, -_(Failed to connect to remote libvirt URI %s), uri); +_(Failed to connect to remote libvirt URI %s), dconnuri); return -1; } -- 1.7.3.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/6] Add API for duplicating a socket/client file descriptor
On 08/15/2011 06:57 AM, Jiri Denemark wrote: Except you missed one change I had pointed out there: bootstrap.conf needs to list 'fcntl' in the list of gnulib modules (without it, gnulib doesn't guarantee that F_DUPFD_CLOEXEC will work on mingw). Hmm, I saw fcntl-h in bootstrap.conf, is that something else? fcntl-h: provides replacement fcntl.h, which guarantees definitions for several useful O_* constants. fcntl: depends on fcntl-h, and additionally provides replacement fcntl(), F_*, and FD_* constants that can be portably implemented (not all F_* flags can be ported to mingw - F_GETFL being a notable omission, unfortunately). -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: Get memory balloon info correctly for text monitor
* src/qemu/qemu_monitor_text.c: BALLOON_PREFIX was defined as balloon: actual=, which cause actual= is stripped early before the real parsing. This patch changes BALLOON_PREFIX into balloon: , and modifies related functions, also renames qemuMonitorParseExtraBalloonInfo to qemuMonitorParseBalloonInfo, as after the changing, it parses all the info returned by info balloon. --- src/qemu/qemu_monitor_text.c | 47 + 1 files changed, 29 insertions(+), 18 deletions(-) diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 7bf733d..d17d863 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -547,8 +547,12 @@ static int parseMemoryStat(char **text, unsigned int tag, return 0; } -/* Convert bytes to kilobytes for libvirt */ switch (tag) { +/* Convert megabytes to kilobytes for libvirt */ +case VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON: +value = value 10; +break; +/* Convert bytes to kilobytes for libvirt */ case VIR_DOMAIN_MEMORY_STAT_SWAP_IN: case VIR_DOMAIN_MEMORY_STAT_SWAP_OUT: case VIR_DOMAIN_MEMORY_STAT_UNUSED: @@ -565,15 +569,17 @@ static int parseMemoryStat(char **text, unsigned int tag, /* The reply from the 'info balloon' command may contain additional memory * statistics in the form: '[,tag=val]*' */ -static int qemuMonitorParseExtraBalloonInfo(char *text, -virDomainMemoryStatPtr stats, -unsigned int nr_stats) +static int qemuMonitorParseBalloonInfo(char *text, + virDomainMemoryStatPtr stats, + unsigned int nr_stats) { char *p = text; unsigned int nr_stats_found = 0; while (*p nr_stats_found nr_stats) { -if (parseMemoryStat(p, VIR_DOMAIN_MEMORY_STAT_SWAP_IN, +if (parseMemoryStat(p, VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON, +actual=, stats[nr_stats_found]) || +parseMemoryStat(p, VIR_DOMAIN_MEMORY_STAT_SWAP_IN, ,mem_swapped_in=, stats[nr_stats_found]) || parseMemoryStat(p, VIR_DOMAIN_MEMORY_STAT_SWAP_OUT, ,mem_swapped_out=, stats[nr_stats_found]) || @@ -584,9 +590,7 @@ static int qemuMonitorParseExtraBalloonInfo(char *text, parseMemoryStat(p, VIR_DOMAIN_MEMORY_STAT_UNUSED, ,free_mem=, stats[nr_stats_found]) || parseMemoryStat(p, VIR_DOMAIN_MEMORY_STAT_AVAILABLE, -,total_mem=, stats[nr_stats_found]) || -parseMemoryStat(p, VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON, -,actual=, stats[nr_stats_found])) +,total_mem=, stats[nr_stats_found])) nr_stats_found++; /* Skip to the next label. When *p is ',' the last match attempt @@ -602,7 +606,7 @@ static int qemuMonitorParseExtraBalloonInfo(char *text, /* The reply from QEMU contains 'ballon: actual=421' where value is in MB */ -#define BALLOON_PREFIX balloon: actual= +#define BALLOON_PREFIX balloon: /* * Returns: 0 if balloon not supported, +1 if balloon query worked @@ -625,13 +629,22 @@ int qemuMonitorTextGetBalloonInfo(qemuMonitorPtr mon, unsigned int memMB; char *end; offset += strlen(BALLOON_PREFIX); -if (virStrToLong_ui(offset, end, 10, memMB) 0) { -qemuReportError(VIR_ERR_OPERATION_FAILED, -_(could not parse memory balloon allocation from '%s'), reply); -goto cleanup; + +if (STRPREFIX(offset, actual=)) { +offset += strlen(actual=); + +if (virStrToLong_ui(offset, end, 10, memMB) 0) { +qemuReportError(VIR_ERR_OPERATION_FAILED, +_(could not parse memory balloon allocation from '%s'), reply); +goto cleanup; +} +*currmem = memMB * 1024; +ret = 1; +} else { +qemuReportError(VIR_ERR_INTERNAL_ERROR, +_(unexpected balloon information '%s'), reply); +ret = -1; } -*currmem = memMB * 1024; -ret = 1; } else { /* We don't raise an error here, since its to be expected that * many QEMU's don't support ballooning @@ -660,9 +673,7 @@ int qemuMonitorTextGetMemoryStats(qemuMonitorPtr mon, if ((offset = strstr(reply, BALLOON_PREFIX)) != NULL) { offset += strlen(BALLOON_PREFIX); -if ((offset = strchr(offset, ',')) != NULL) { -ret = qemuMonitorParseExtraBalloonInfo(offset, stats, nr_stats); -} +ret = qemuMonitorParseBalloonInfo(offset, stats, nr_stats); } VIR_FREE(reply);
Re: [libvirt] [PATCH] qemu: support event_idx parameter for virtio disk and net devices
On Sun, Aug 14, 2011 at 03:43:52AM -0400, Laine Stump wrote: In some versions of qemu, both virtio-blk-pci and virtio-net-pci devices can have an event_idx setting that determines some details of event processing. When it is enabled, it reduces the number of interrupts and exits for the guest. qemu will automatically enable this feature when it is available, but there may be cases where this new feature could actually make performance worse (NB: no such case has been found so far). As a safety switch in case such a situation is encountered in the field, this patch adds a new attribute event_idx to the driver element of both disk and interface devices. event_idx can be set to on (to force event_idx on in case qemu has it disabled by default) or off (for force event_idx off). In the case that event_idx support isn't present in qemu, the attribute is ignored (this on the advice of the qemu developer). This smells like an API available just as a safety belt for code not fully tested in the field. So I have the same dislike for consolidating this as a full long term libvirt API. But that's not the first one, and I'm afraid it's not the last one, and it affects only the XML domain with an extra attribute so the damage is limited. [...] + li +The optional codeevent_idx/code attribute controls +some aspects of device event processing. The value can be +either 'on' or 'off' - if it is on, it will reduce the +number of interupts and exits for the guest. The default +is determined by QEMU; usually if the feature is +supported, default is on. In case there is a situation +where this behavior is suboptimal, this attribute provides +a way to force the feature off. +span class=sinceSince 0.9.5 (QEMU and KVM only)/span +bIn general you should leave this option alone, unless you +are very certain you know what you are doing./b + /li Okay at least it's clear in the documentation Code looks correct, there is one example trying to test both values and for the 2 use cases, then fine, ACK, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Get memory balloon info correctly for text monitor
于 2011年08月15日 21:58, Osier Yang 写道: * src/qemu/qemu_monitor_text.c: BALLOON_PREFIX was defined as balloon: actual=, which cause actual= is stripped early before the real parsing. This patch changes BALLOON_PREFIX into balloon: , and modifies related functions, also renames qemuMonitorParseExtraBalloonInfo to qemuMonitorParseBalloonInfo, as after the changing, it parses all the info returned by info balloon. Forgot to mention the problem, e.g. virsh dommemstat $domain returns empty result. --- src/qemu/qemu_monitor_text.c | 47 + 1 files changed, 29 insertions(+), 18 deletions(-) diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 7bf733d..d17d863 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -547,8 +547,12 @@ static int parseMemoryStat(char **text, unsigned int tag, return 0; } -/* Convert bytes to kilobytes for libvirt */ switch (tag) { +/* Convert megabytes to kilobytes for libvirt */ +case VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON: +value = value 10; +break; +/* Convert bytes to kilobytes for libvirt */ case VIR_DOMAIN_MEMORY_STAT_SWAP_IN: case VIR_DOMAIN_MEMORY_STAT_SWAP_OUT: case VIR_DOMAIN_MEMORY_STAT_UNUSED: @@ -565,15 +569,17 @@ static int parseMemoryStat(char **text, unsigned int tag, /* The reply from the 'info balloon' command may contain additional memory * statistics in the form: '[,tag=val]*' */ -static int qemuMonitorParseExtraBalloonInfo(char *text, -virDomainMemoryStatPtr stats, -unsigned int nr_stats) +static int qemuMonitorParseBalloonInfo(char *text, + virDomainMemoryStatPtr stats, + unsigned int nr_stats) { char *p = text; unsigned int nr_stats_found = 0; while (*p nr_stats_found nr_stats) { -if (parseMemoryStat(p, VIR_DOMAIN_MEMORY_STAT_SWAP_IN, +if (parseMemoryStat(p, VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON, +actual=, stats[nr_stats_found]) || +parseMemoryStat(p, VIR_DOMAIN_MEMORY_STAT_SWAP_IN, ,mem_swapped_in=, stats[nr_stats_found]) || parseMemoryStat(p, VIR_DOMAIN_MEMORY_STAT_SWAP_OUT, ,mem_swapped_out=, stats[nr_stats_found]) || @@ -584,9 +590,7 @@ static int qemuMonitorParseExtraBalloonInfo(char *text, parseMemoryStat(p, VIR_DOMAIN_MEMORY_STAT_UNUSED, ,free_mem=, stats[nr_stats_found]) || parseMemoryStat(p, VIR_DOMAIN_MEMORY_STAT_AVAILABLE, -,total_mem=, stats[nr_stats_found]) || -parseMemoryStat(p, VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON, -,actual=, stats[nr_stats_found])) +,total_mem=, stats[nr_stats_found])) nr_stats_found++; /* Skip to the next label. When *p is ',' the last match attempt @@ -602,7 +606,7 @@ static int qemuMonitorParseExtraBalloonInfo(char *text, /* The reply from QEMU contains 'ballon: actual=421' where value is in MB */ -#define BALLOON_PREFIX balloon: actual= +#define BALLOON_PREFIX balloon: /* * Returns: 0 if balloon not supported, +1 if balloon query worked @@ -625,13 +629,22 @@ int qemuMonitorTextGetBalloonInfo(qemuMonitorPtr mon, unsigned int memMB; char *end; offset += strlen(BALLOON_PREFIX); -if (virStrToLong_ui(offset, end, 10, memMB) 0) { -qemuReportError(VIR_ERR_OPERATION_FAILED, -_(could not parse memory balloon allocation from '%s'), reply); -goto cleanup; + +if (STRPREFIX(offset, actual=)) { +offset += strlen(actual=); + +if (virStrToLong_ui(offset, end, 10, memMB) 0) { +qemuReportError(VIR_ERR_OPERATION_FAILED, +_(could not parse memory balloon allocation from '%s'), reply); +goto cleanup; +} +*currmem = memMB * 1024; +ret = 1; +} else { +qemuReportError(VIR_ERR_INTERNAL_ERROR, +_(unexpected balloon information '%s'), reply); +ret = -1; } -*currmem = memMB * 1024; -ret = 1; } else { /* We don't raise an error here, since its to be expected that * many QEMU's don't support ballooning @@ -660,9 +673,7 @@ int qemuMonitorTextGetMemoryStats(qemuMonitorPtr mon, if ((offset = strstr(reply, BALLOON_PREFIX)) != NULL) { offset += strlen(BALLOON_PREFIX); -if
Re: [libvirt] [PATCH] qemu: error message should show uri instead of (null)
On Mon, Aug 15, 2011 at 14:58:47 +0200, Peter Krempa wrote: Fix pointer for error message uri if domain migration fails. BZ# 730244 --- src/qemu/qemu_migration.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 4d0e062..d900020 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2079,7 +2079,7 @@ static int doPeer2PeerMigrate(struct qemud_driver *driver, qemuDomainObjExitRemoteWithDriver(driver, vm); if (dconn == NULL) { qemuReportError(VIR_ERR_OPERATION_FAILED, -_(Failed to connect to remote libvirt URI %s), uri); +_(Failed to connect to remote libvirt URI %s), dconnuri); return -1; } Oops, ACK and pushed. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/6] Use fd: protocol for migration
On Mon, Aug 15, 2011 at 09:58:10 +0200, Jiri Denemark wrote: Since qemu doesn't give us any reasonable errors when migration fails because of connection issues, we now create a connection to destination qemu ourselves and just pass the created socket to qemu. Daniel P. Berrange (1): Add API for duplicating a socket/client file descriptor Jiri Denemark (5): Add backlog parameter to virNetSocketListen Support changing UNIX socket owner in virNetSocketNewListenUNIX qemu: Refactor do{Tunnel,Native}Migrate functions qemu: Use virNetSocket for tunneled migration qemu: Use fd: protocol for migration src/qemu/qemu_migration.c | 541 + src/rpc/virnetclient.c| 20 ++ src/rpc/virnetclient.h|3 + src/rpc/virnetserverservice.c |6 +- src/rpc/virnetsocket.c| 29 ++- src/rpc/virnetsocket.h|5 +- tests/virnetsockettest.c | 10 +- 7 files changed, 336 insertions(+), 278 deletions(-) I modified the patches according to comments, added fcntl into boostrap.conf and pushed the series. Thanks for reviews. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: error message should show uri instead of (null)
On Mon, Aug 15, 2011 at 02:58:47PM +0200, Peter Krempa wrote: Fix pointer for error message uri if domain migration fails. BZ# 730244 --- src/qemu/qemu_migration.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 4d0e062..d900020 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2079,7 +2079,7 @@ static int doPeer2PeerMigrate(struct qemud_driver *driver, qemuDomainObjExitRemoteWithDriver(driver, vm); if (dconn == NULL) { qemuReportError(VIR_ERR_OPERATION_FAILED, -_(Failed to connect to remote libvirt URI %s), uri); +_(Failed to connect to remote libvirt URI %s), dconnuri); return -1; } Seems to me that if dconnuri was NULL we would certainly hit dconn == NULL as a result. That said we were printing the wrong URL so that's still an improvement :-) maybe use (dconnuri ? dconnuri : NULL) or test and make 2 spearate messages, but that probably excessive :-) Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: error message should show uri instead of (null)
On 08/15/2011 07:43 AM, Daniel Veillard wrote: qemuReportError(VIR_ERR_OPERATION_FAILED, -_(Failed to connect to remote libvirt URI %s), uri); +_(Failed to connect to remote libvirt URI %s), dconnuri); return -1; } Seems to me that if dconnuri was NULL we would certainly hit dconn == NULL as a result. That said we were printing the wrong URL so that's still an improvement :-) maybe use (dconnuri ? dconnuri : NULL) We already have a macro for that: NULLSTR(dconnuri). or test and make 2 spearate messages, but that probably excessive :-) Agree that 2 messages is overkill in this case. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: error message should show uri instead of (null)
于 2011年08月15日 21:43, Daniel Veillard 写道: On Mon, Aug 15, 2011 at 02:58:47PM +0200, Peter Krempa wrote: Fix pointer for error message uri if domain migration fails. BZ# 730244 --- src/qemu/qemu_migration.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 4d0e062..d900020 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2079,7 +2079,7 @@ static int doPeer2PeerMigrate(struct qemud_driver *driver, qemuDomainObjExitRemoteWithDriver(driver, vm); if (dconn == NULL) { qemuReportError(VIR_ERR_OPERATION_FAILED, -_(Failed to connect to remote libvirt URI %s), uri); +_(Failed to connect to remote libvirt URI %s), dconnuri); return -1; } Seems to me that if dconnuri was NULL we would certainly hit dconn == NULL as a result. That said we were printing the wrong URL so that's still an improvement :-) maybe use (dconnuri ? dconnuri : NULL) This should be NULLSTR(dconnuri). or test and make 2 spearate messages, but that probably excessive :-) Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Get memory balloon info correctly for text monitor
On 08/15/2011 08:23 AM, Osier Yang wrote: 于 2011年08月15日 21:58, Osier Yang 写道: * src/qemu/qemu_monitor_text.c: BALLOON_PREFIX was defined as balloon: actual=, which cause actual= is stripped early before the real parsing. This patch changes BALLOON_PREFIX into balloon: , and modifies related functions, also renames qemuMonitorParseExtraBalloonInfo to qemuMonitorParseBalloonInfo, as after the changing, it parses all the info returned by info balloon. Forgot to mention the problem, e.g. virsh dommemstat $domain returns empty result. That is because qemu has disabled stats reporting and so the extra fields are not present in the info balloon response. -- Adam Litke IBM Linux Technology Center -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Get memory balloon info correctly for text monitor
Hi Osier, Just to be clear, this is a cleanup not a bugfix right? The current code should be working properly as written. On 08/15/2011 08:58 AM, Osier Yang wrote: * src/qemu/qemu_monitor_text.c: BALLOON_PREFIX was defined as balloon: actual=, which cause actual= is stripped early before the real parsing. This patch changes BALLOON_PREFIX into balloon: , and modifies related functions, also renames qemuMonitorParseExtraBalloonInfo to qemuMonitorParseBalloonInfo, as after the changing, it parses all the info returned by info balloon. --- src/qemu/qemu_monitor_text.c | 47 + 1 files changed, 29 insertions(+), 18 deletions(-) diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 7bf733d..d17d863 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -547,8 +547,12 @@ static int parseMemoryStat(char **text, unsigned int tag, return 0; } -/* Convert bytes to kilobytes for libvirt */ switch (tag) { +/* Convert megabytes to kilobytes for libvirt */ +case VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON: +value = value 10; +break; +/* Convert bytes to kilobytes for libvirt */ case VIR_DOMAIN_MEMORY_STAT_SWAP_IN: case VIR_DOMAIN_MEMORY_STAT_SWAP_OUT: case VIR_DOMAIN_MEMORY_STAT_UNUSED: @@ -565,15 +569,17 @@ static int parseMemoryStat(char **text, unsigned int tag, /* The reply from the 'info balloon' command may contain additional memory * statistics in the form: '[,tag=val]*' */ -static int qemuMonitorParseExtraBalloonInfo(char *text, -virDomainMemoryStatPtr stats, -unsigned int nr_stats) +static int qemuMonitorParseBalloonInfo(char *text, + virDomainMemoryStatPtr stats, + unsigned int nr_stats) { char *p = text; unsigned int nr_stats_found = 0; while (*p nr_stats_found nr_stats) { -if (parseMemoryStat(p, VIR_DOMAIN_MEMORY_STAT_SWAP_IN, +if (parseMemoryStat(p, VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON, +actual=, stats[nr_stats_found]) || +parseMemoryStat(p, VIR_DOMAIN_MEMORY_STAT_SWAP_IN, ,mem_swapped_in=, stats[nr_stats_found]) || parseMemoryStat(p, VIR_DOMAIN_MEMORY_STAT_SWAP_OUT, ,mem_swapped_out=, stats[nr_stats_found]) || According to this code (and actual monitor behavior) 'actual' always appears and has to come first. Therefore, I would still parse the 'actual' stat outside of the while loop. @@ -584,9 +590,7 @@ static int qemuMonitorParseExtraBalloonInfo(char *text, parseMemoryStat(p, VIR_DOMAIN_MEMORY_STAT_UNUSED, ,free_mem=, stats[nr_stats_found]) || parseMemoryStat(p, VIR_DOMAIN_MEMORY_STAT_AVAILABLE, -,total_mem=, stats[nr_stats_found]) || -parseMemoryStat(p, VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON, -,actual=, stats[nr_stats_found])) +,total_mem=, stats[nr_stats_found])) nr_stats_found++; /* Skip to the next label. When *p is ',' the last match attempt @@ -602,7 +606,7 @@ static int qemuMonitorParseExtraBalloonInfo(char *text, /* The reply from QEMU contains 'ballon: actual=421' where value is in MB */ -#define BALLOON_PREFIX balloon: actual= +#define BALLOON_PREFIX balloon: /* * Returns: 0 if balloon not supported, +1 if balloon query worked @@ -625,13 +629,22 @@ int qemuMonitorTextGetBalloonInfo(qemuMonitorPtr mon, unsigned int memMB; char *end; offset += strlen(BALLOON_PREFIX); -if (virStrToLong_ui(offset, end, 10, memMB) 0) { -qemuReportError(VIR_ERR_OPERATION_FAILED, -_(could not parse memory balloon allocation from '%s'), reply); -goto cleanup; + +if (STRPREFIX(offset, actual=)) { +offset += strlen(actual=); + +if (virStrToLong_ui(offset, end, 10, memMB) 0) { +qemuReportError(VIR_ERR_OPERATION_FAILED, +_(could not parse memory balloon allocation from '%s'), reply); +goto cleanup; +} +*currmem = memMB * 1024; +ret = 1; +} else { +qemuReportError(VIR_ERR_INTERNAL_ERROR, +_(unexpected balloon information '%s'), reply); +ret = -1; } -*currmem = memMB * 1024; -ret = 1; } else { /* We don't raise an error here, since its to be expected that * many QEMU's don't
Re: [libvirt] [PATCH] qemu: Get memory balloon info correctly for text monitor
On Mon, Aug 15, 2011 at 11:27:43AM -0500, Adam Litke wrote: On 08/15/2011 08:23 AM, Osier Yang wrote: 于 2011年08月15日 21:58, Osier Yang 写道: * src/qemu/qemu_monitor_text.c: BALLOON_PREFIX was defined as balloon: actual=, which cause actual= is stripped early before the real parsing. This patch changes BALLOON_PREFIX into balloon: , and modifies related functions, also renames qemuMonitorParseExtraBalloonInfo to qemuMonitorParseBalloonInfo, as after the changing, it parses all the info returned by info balloon. Forgot to mention the problem, e.g. virsh dommemstat $domain returns empty result. That is because qemu has disabled stats reporting and so the extra fields are not present in the info balloon response. I'd completely forgotten about this problem. We really should try to get this fixed renabled in QEMU sometime in the not too distant future. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/4 v2] pci: Move some pci sriov helper code out of node device driver to util/pci
On 8/15/11 3:58 AM, Stefan Berger stef...@linux.vnet.ibm.com wrote: On 08/12/2011 07:14 PM, Roopa Prabhu wrote: -get_physical_function(sysfs_path, d); -get_virtual_functions(sysfs_path, d); +if (!pciGetPhysicalFunction(sysfs_path,d-pci_dev.physical_function)) +d-pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION; + +if (!pciGetVirtualFunctions(sysfs_path,d-pci_dev.virtual_functions, +d-pci_dev.num_virtual_functions) || +d-pci_dev.num_virtual_functions 0) +d-pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION; VIR_FREE(sysfs_path); [...] diff --git a/src/util/pci.h b/src/util/pci.h index a351baf..367881e 100644 --- a/src/util/pci.h +++ b/src/util/pci.h @@ -27,6 +27,13 @@ typedef struct _pciDevice pciDevice; typedef struct _pciDeviceList pciDeviceList; +struct pci_config_address { +unsigned int domain; +unsigned int bus; +unsigned int slot; +unsigned int function; +}; + pciDevice *pciGetDevice (unsigned domain, unsigned bus, unsigned slot, @@ -74,4 +81,22 @@ int pciDeviceIsAssignable(pciDevice *dev, int strict_acs_check); int pciWaitForDeviceCleanup(pciDevice *dev, const char *matcher); +#ifdef __linux__ + +# define pciGetPhysicalFunction(s,a) pciGetPhysicalFunctionLinux(s,a) +int pciGetPhysicalFunctionLinux(const char *sysfs_path, +struct pci_config_address **phys_fn); + +# define pciGetVirtualFunctions(s,a,n) pciGetVirtualFunctionsLinux(s,a,n) +int pciGetVirtualFunctionsLinux(const char *sysfs_path, +struct pci_config_address ***virtual_functions, +unsigned int *num_virtual_functions); + +#else /* __linux__ */ + +# define pciGetPhysicalFunction(s,a) +# define pciGetVirtualFunctions(s,a,n) + I don't think these #defines will produce compilable code if they are used above. You'll probably have to implement functions for them. Ok, I actually moved them from node_device driver as is with only name changes. I can implement them returning -1 for non __linux__ Will resubmit. Thanks. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Ensure client streams are closed when marking a client for close
From: Daniel P. Berrange berra...@redhat.com Every active stream results in a reference being held on the virNetServerClientPtr object. This meant that if a client quit with any streams active, although all I/O was stopped the virNetServerClientPtr object would leak. This causes libvirtd to leak any file handles associated with open streams when a client quit To fix this, when we call virNetServerClientClose there is a callback invoked which lets the daemon release the streams and thus the extra references * daemon/remote.c: Add a hook to close all streams * src/rpc/virnetserverclient.c, src/rpc/virnetserverclient.h: Allow registration of a hook to trigger when closing client --- daemon/remote.c | 13 ++--- src/rpc/virnetserverclient.c | 21 + src/rpc/virnetserverclient.h |5 + 3 files changed, 36 insertions(+), 3 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index ec261e2..0f088c6 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -419,8 +419,6 @@ static void remoteClientFreeFunc(void *data) { struct daemonClientPrivate *priv = data; -daemonRemoveAllClientStreams(priv-streams); - /* Deregister event delivery callback */ if (priv-conn) { int i; @@ -441,6 +439,13 @@ static void remoteClientFreeFunc(void *data) } +static void remoteClientCloseFunc(virNetServerClientPtr client) +{ +struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client); + +daemonRemoveAllClientStreams(priv-streams); +} + int remoteClientInitHook(virNetServerPtr srv ATTRIBUTE_UNUSED, virNetServerClientPtr client) @@ -462,7 +467,9 @@ int remoteClientInitHook(virNetServerPtr srv ATTRIBUTE_UNUSED, for (i = 0 ; i VIR_DOMAIN_EVENT_ID_LAST ; i++) priv-domainEventCallbackID[i] = -1; -virNetServerClientSetPrivateData(client, priv, remoteClientFreeFunc); +virNetServerClientSetPrivateData(client, priv, + remoteClientFreeFunc); +virNetServerClientSetCloseHook(client, remoteClientCloseFunc); return 0; } diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index e246fa5..a73b06d 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -97,6 +97,7 @@ struct _virNetServerClient void *privateData; virNetServerClientFreeFunc privateDataFreeFunc; +virNetServerClientCloseFunc privateDataCloseFunc; }; @@ -492,6 +493,15 @@ void *virNetServerClientGetPrivateData(virNetServerClientPtr client) } +void virNetServerClientSetCloseHook(virNetServerClientPtr client, +virNetServerClientCloseFunc cf) +{ +virNetServerClientLock(client); +client-privateDataCloseFunc = cf; +virNetServerClientUnlock(client); +} + + void virNetServerClientSetDispatcher(virNetServerClientPtr client, virNetServerClientDispatchFunc func, void *opaque) @@ -560,6 +570,8 @@ void virNetServerClientFree(virNetServerClientPtr client) */ void virNetServerClientClose(virNetServerClientPtr client) { +virNetServerClientCloseFunc cf; + virNetServerClientLock(client); VIR_DEBUG(client=%p refs=%d, client, client-refs); if (!client-sock) { @@ -567,6 +579,15 @@ void virNetServerClientClose(virNetServerClientPtr client) return; } +if (client-privateDataCloseFunc) { +cf = client-privateDataCloseFunc; +client-refs++; +virNetServerClientUnlock(client); +(cf)(client); +virNetServerClientLock(client); +client-refs--; +} + /* Do now, even though we don't close the socket * until end, to ensure we don't get invoked * again due to tls shutdown */ diff --git a/src/rpc/virnetserverclient.h b/src/rpc/virnetserverclient.h index 3d2e1fb..bedb179 100644 --- a/src/rpc/virnetserverclient.h +++ b/src/rpc/virnetserverclient.h @@ -82,6 +82,11 @@ void virNetServerClientSetPrivateData(virNetServerClientPtr client, virNetServerClientFreeFunc ff); void *virNetServerClientGetPrivateData(virNetServerClientPtr client); +typedef void (*virNetServerClientCloseFunc)(virNetServerClientPtr client); + +void virNetServerClientSetCloseHook(virNetServerClientPtr client, +virNetServerClientCloseFunc cf); + void virNetServerClientSetDispatcher(virNetServerClientPtr client, virNetServerClientDispatchFunc func, void *opaque); -- 1.7.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Ensure stream is aborted when exiting console
From: Daniel P. Berrange berra...@redhat.com After running 'virsh console' in interactive mode, there was a missing call to virStreamAbort, which meant the server kept the stream resources open * tools/console.c: Abort stream when exiting --- tools/console.c |6 -- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/tools/console.c b/tools/console.c index 171ebc9..82ef537 100644 --- a/tools/console.c +++ b/tools/console.c @@ -90,9 +90,11 @@ static void virConsoleShutdown(virConsolePtr con) { con-quit = true; -virStreamEventRemoveCallback(con-st); -if (con-st) +if (con-st) { +virStreamEventRemoveCallback(con-st); +virStreamAbort(con-st); virStreamFree(con-st); +} if (con-stdinWatch != -1) virEventRemoveHandle(con-stdinWatch); if (con-stdoutWatch != -1) -- 1.7.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/4 v2] pci: Move some pci sriov helper code out of node device driver to util/pci
On 8/15/11 3:47 AM, Stefan Berger stef...@linux.vnet.ibm.com wrote: On 08/12/2011 07:14 PM, Roopa Prabhu wrote: From: Roopa Prabhuropra...@cisco.com This patch moves some of the sriov related pci code from node_device driver to src/util/pci.[ch]. Some functions had to go thru name and argument list change to accommodate the move. Signed-off-by: Roopa Prabhuropra...@cisco.com Signed-off-by: Christian Benvenutibe...@cisco.com Signed-off-by: David Wangdwa...@cisco.com --- src/Makefile.am |5 + src/conf/node_device_conf.c |1 src/conf/node_device_conf.h |7 - src/node_device/node_device_driver.h | 14 -- src/node_device/node_device_hal.c | 10 + src/node_device/node_device_linux_sysfs.c | 191 src/node_device/node_device_udev.c| 10 + src/util/pci.c| 196 + src/util/pci.h| 25 9 files changed, 242 insertions(+), 217 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index 009ff25..4246823 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -953,7 +953,10 @@ libvirt_driver_nodedev_la_SOURCES = $(NODE_DEVICE_DRIVER_SOURCES) libvirt_driver_nodedev_la_CFLAGS = \ -I@top_srcdir@/src/conf $(AM_CFLAGS) libvirt_driver_nodedev_la_LDFLAGS = $(AM_LDFLAGS) -libvirt_driver_nodedev_la_LIBADD = +libvirt_driver_nodedev_la_LIBADD = \ + libvirt_util.la \ + ../gnulib/lib/libgnu.la + if HAVE_HAL libvirt_driver_nodedev_la_SOURCES += $(NODE_DEVICE_DRIVER_HAL_SOURCES) libvirt_driver_nodedev_la_CFLAGS += $(HAL_CFLAGS) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index dde2921..548bbff 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -36,6 +36,7 @@ #include util.h #include buf.h #include uuid.h +#include pci.h #define VIR_FROM_THIS VIR_FROM_NODEDEV diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index cef86d4..17be031 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -82,13 +82,6 @@ enum virNodeDevPCICapFlags { VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION = (1 1), }; -struct pci_config_address { -unsigned int domain; -unsigned int bus; -unsigned int slot; -unsigned int function; -}; - typedef struct _virNodeDevCapsDef virNodeDevCapsDef; typedef virNodeDevCapsDef *virNodeDevCapsDefPtr; struct _virNodeDevCapsDef { diff --git a/src/node_device/node_device_driver.h b/src/node_device/node_device_driver.h index 08779b1..673e95b 100644 --- a/src/node_device/node_device_driver.h +++ b/src/node_device/node_device_driver.h @@ -37,10 +37,6 @@ # define LINUX_SYSFS_VPORT_CREATE_POSTFIX /vport_create # define LINUX_SYSFS_VPORT_DELETE_POSTFIX /vport_delete -# define SRIOV_FOUND 0 -# define SRIOV_NOT_FOUND 1 -# define SRIOV_ERROR -1 - # define LINUX_NEW_DEVICE_WAIT_TIME 60 # ifdef HAVE_HAL @@ -63,14 +59,6 @@ int check_fc_host_linux(union _virNodeDevCapData *d); # define check_vport_capable(d) check_vport_capable_linux(d) int check_vport_capable_linux(union _virNodeDevCapData *d); -# define get_physical_function(s,d) get_physical_function_linux(s,d) -int get_physical_function_linux(const char *sysfs_path, -union _virNodeDevCapData *d); - -# define get_virtual_functions(s,d) get_virtual_functions_linux(s,d) -int get_virtual_functions_linux(const char *sysfs_path, -union _virNodeDevCapData *d); - # define read_wwn(host, file, wwn) read_wwn_linux(host, file, wwn) int read_wwn_linux(int host, const char *file, char **wwn); @@ -78,8 +66,6 @@ int read_wwn_linux(int host, const char *file, char **wwn); # define check_fc_host(d) (-1) # define check_vport_capable(d)(-1) -# define get_physical_function(sysfs_path, d) -# define get_virtual_functions(sysfs_path, d) # define read_wwn(host, file, wwn) # endif /* __linux__ */ diff --git a/src/node_device/node_device_hal.c b/src/node_device/node_device_hal.c index 421f5ad..481be97 100644 --- a/src/node_device/node_device_hal.c +++ b/src/node_device/node_device_hal.c @@ -35,6 +35,7 @@ #include datatypes.h #include memory.h #include uuid.h +#include pci.h #include logging.h #include node_device_driver.h @@ -146,8 +147,13 @@ static int gather_pci_cap(LibHalContext *ctx, const char *udi, (void)virStrToLong_ui(p+1,p, 16,d-pci_dev.function); } -get_physical_function(sysfs_path, d); -get_virtual_functions(sysfs_path, d); +if (!pciGetPhysicalFunction(sysfs_path,d-pci_dev.physical_function)) +d-pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION; + +if
Re: [libvirt] [PATCH] qemu: Get memory balloon info correctly for text monitor
On 08/15/2011 11:50 AM, Daniel P. Berrange wrote: On Mon, Aug 15, 2011 at 11:27:43AM -0500, Adam Litke wrote: On 08/15/2011 08:23 AM, Osier Yang wrote: 于 2011年08月15日 21:58, Osier Yang 写道: * src/qemu/qemu_monitor_text.c: BALLOON_PREFIX was defined as balloon: actual=, which cause actual= is stripped early before the real parsing. This patch changes BALLOON_PREFIX into balloon: , and modifies related functions, also renames qemuMonitorParseExtraBalloonInfo to qemuMonitorParseBalloonInfo, as after the changing, it parses all the info returned by info balloon. Forgot to mention the problem, e.g. virsh dommemstat $domain returns empty result. That is because qemu has disabled stats reporting and so the extra fields are not present in the info balloon response. I'd completely forgotten about this problem. We really should try to get this fixed renabled in QEMU sometime in the not too distant future. I agree. The problem is that qemu lacks a proper interface for asynchronous commands so an unresponsive guest could freeze the monitor. QMP is undergoing a significant overhaul as a result of the new QAPI framework. It is my understanding that this new framework will provide a robust async interface, allowing us to re-enable balloon stats. -- Adam Litke IBM Linux Technology Center -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] Ensure client streams are closed when marking a client for close
From: Daniel P. Berrange berra...@redhat.com NB, previous patch was borked due to bad rebase Every active stream results in a reference being held on the virNetServerClientPtr object. This meant that if a client quit with any streams active, although all I/O was stopped the virNetServerClientPtr object would leak. This causes libvirtd to leak any file handles associated with open streams when a client quit To fix this, when we call virNetServerClientClose there is a callback invoked which lets the daemon release the streams and thus the extra references * daemon/remote.c: Add a hook to close all streams * daemon/stream.c, daemon/stream.h: Add API for releasing all streams * src/rpc/virnetserverclient.c, src/rpc/virnetserverclient.h: Allow registration of a hook to trigger when closing client --- daemon/remote.c | 11 ++- daemon/stream.c | 38 -- daemon/stream.h |3 +++ src/rpc/virnetserverclient.c | 21 + src/rpc/virnetserverclient.h |5 + 5 files changed, 71 insertions(+), 7 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 939044c..0f088c6 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -439,6 +439,13 @@ static void remoteClientFreeFunc(void *data) } +static void remoteClientCloseFunc(virNetServerClientPtr client) +{ +struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client); + +daemonRemoveAllClientStreams(priv-streams); +} + int remoteClientInitHook(virNetServerPtr srv ATTRIBUTE_UNUSED, virNetServerClientPtr client) @@ -460,7 +467,9 @@ int remoteClientInitHook(virNetServerPtr srv ATTRIBUTE_UNUSED, for (i = 0 ; i VIR_DOMAIN_EVENT_ID_LAST ; i++) priv-domainEventCallbackID[i] = -1; -virNetServerClientSetPrivateData(client, priv, remoteClientFreeFunc); +virNetServerClientSetPrivateData(client, priv, + remoteClientFreeFunc); +virNetServerClientSetCloseHook(client, remoteClientCloseFunc); return 0; } diff --git a/daemon/stream.c b/daemon/stream.c index 4a8f1ee..c6865a8 100644 --- a/daemon/stream.c +++ b/daemon/stream.c @@ -334,13 +334,17 @@ int daemonFreeClientStream(virNetServerClientPtr client, msg = stream-rx; while (msg) { virNetMessagePtr tmp = msg-next; -/* Send a dummy reply to free up 'msg' unblock client rx */ -memset(msg, 0, sizeof(*msg)); -msg-header.type = VIR_NET_REPLY; -if (virNetServerClientSendMessage(client, msg) 0) { -virNetServerClientImmediateClose(client); +if (client) { +/* Send a dummy reply to free up 'msg' unblock client rx */ +memset(msg, 0, sizeof(*msg)); +msg-header.type = VIR_NET_REPLY; +if (virNetServerClientSendMessage(client, msg) 0) { +virNetServerClientImmediateClose(client); +virNetMessageFree(msg); +ret = -1; +} +} else { virNetMessageFree(msg); -ret = -1; } msg = tmp; } @@ -441,6 +445,28 @@ daemonRemoveClientStream(virNetServerClientPtr client, } +void +daemonRemoveAllClientStreams(daemonClientStream *stream) +{ +daemonClientStream *tmp; + +VIR_DEBUG(stream=%p, stream); + +while (stream) { +tmp = stream-next; + +if (!stream-closed) { +virStreamEventRemoveCallback(stream-st); +virStreamAbort(stream-st); +} + +daemonFreeClientStream(NULL, stream); + +VIR_DEBUG(next stream=%p, stream); +stream = tmp; +} +} + /* * Returns: * -1 if fatal error occurred diff --git a/daemon/stream.h b/daemon/stream.h index 626ae60..7c2d8dc 100644 --- a/daemon/stream.h +++ b/daemon/stream.h @@ -45,4 +45,7 @@ int daemonRemoveClientStream(virNetServerClientPtr client, daemonClientStream *stream); +void +daemonRemoveAllClientStreams(daemonClientStream *stream); + #endif /* __LIBVIRTD_STREAM_H__ */ diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index e246fa5..a73b06d 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -97,6 +97,7 @@ struct _virNetServerClient void *privateData; virNetServerClientFreeFunc privateDataFreeFunc; +virNetServerClientCloseFunc privateDataCloseFunc; }; @@ -492,6 +493,15 @@ void *virNetServerClientGetPrivateData(virNetServerClientPtr client) } +void virNetServerClientSetCloseHook(virNetServerClientPtr client, +virNetServerClientCloseFunc cf) +{ +virNetServerClientLock(client); +client-privateDataCloseFunc = cf; +virNetServerClientUnlock(client); +} + + void virNetServerClientSetDispatcher(virNetServerClientPtr client, virNetServerClientDispatchFunc func,
Re: [libvirt] [PATCH 2/4 v2] pci: Add helper functions for sriov devices
On 8/15/11 4:03 AM, Stefan Berger stef...@linux.vnet.ibm.com wrote: On 08/12/2011 07:14 PM, Roopa Prabhu wrote: From: Roopa Prabhuropra...@cisco.com This patch adds the following helper functions: pciDeviceIsVirtualFunction: Function to check if a pci device is a sriov VF pciGetVirtualFunctionIndex: Function to get the VF index of a sriov VF pciDeviceNetName: Function to get the network device name of a pci device pciConfigAddressCompare: Function to compare pci config addresses Signed-off-by: Roopa Prabhuropra...@cisco.com Signed-off-by: Christian Benvenutibe...@cisco.com Signed-off-by: David Wangdwa...@cisco.com --- src/util/pci.c | 118 src/util/pci.h | 15 +++ 2 files changed, 133 insertions(+), 0 deletions(-) diff --git a/src/util/pci.c b/src/util/pci.c index e017db9..558c7ae 100644 --- a/src/util/pci.c +++ b/src/util/pci.c @@ -1875,3 +1875,121 @@ out: return ret; } + +/* + * returns 0 if equal and 1 if not + */ +static int +pciConfigAddressCompare(struct pci_config_address *bdf1, +struct pci_config_address *bdf2) +{ +return !((bdf1-domain == bdf2-domain) + (bdf1-bus == bdf2-bus) + (bdf1-slot == bdf2-slot) + (bdf1-function == bdf2-function)); +} Would it no be more intuitive to call implement a function pciConfigAddressEqual and return '1' in case the addresses are equal? Agree. Will implement it as pciConfigAddressEqual. + +/* + * Returns 1 if vf device is a virtual function, 0 if not, -1 on error + */ +int +pciDeviceIsVirtualFunctionLinux(const char *vf_sysfs_device_link) +{ +char *vf_sysfs_physfn_link = NULL; +int ret = -1; + +if (virAsprintf(vf_sysfs_physfn_link, %s/physfn, +vf_sysfs_device_link) 0) { +virReportOOMError(); +return ret; +} + +ret = virFileExists(vf_sysfs_physfn_link); + +VIR_FREE(vf_sysfs_physfn_link); + +return ret; +} + +/* + * Returns the sriov virtual function index of vf given its pf + */ +int +pciGetVirtualFunctionIndexLinux(const char *pf_sysfs_device_link, +const char *vf_sysfs_device_link, +int *vf_index) +{ +int ret = -1, i; +unsigned int num_virt_fns = 0; +struct pci_config_address *vf_bdf = NULL; +struct pci_config_address **virt_fns = NULL; + +if (pciGetPciConfigAddressFromSysfsDeviceLink(vf_sysfs_device_link, +vf_bdf)) +return ret; + +if (pciGetVirtualFunctionsLinux(pf_sysfs_device_link,virt_fns, +num_virt_fns)) { +VIR_DEBUG(Error getting physical function's '%s' virtual_functions\n, + pf_sysfs_device_link); +goto out; +} + +for (i = 0; i num_virt_fns; i++) { + if (!pciConfigAddressCompare(vf_bdf, virt_fns[i])) { + *vf_index = i; + ret = 0; + break; + } +} + +out: + +/* free virtual functions */ +for (i = 0; i num_virt_fns; i++) + VIR_FREE(virt_fns[i]); + +VIR_FREE(vf_bdf); + +return ret; +} + +/* + * Returns the network device name of a pci device + */ +int +pciDeviceNetNameLinux(char *device_link_sysfs_path, char **netname) +{ + char *pcidev_sysfs_net_path = NULL; + int ret = -1; + DIR *dir = NULL; + struct dirent *entry = NULL; + + if (virBuildPath(pcidev_sysfs_net_path, device_link_sysfs_path, + net) == -1) { + virReportOOMError(); + return -1; + } + + dir = opendir(pcidev_sysfs_net_path); + if (dir == NULL) + goto out; + + while ((entry = readdir(dir))) { +if (!strcmp(entry-d_name, .) || +!strcmp(entry-d_name, ..)) +continue; + +/* Assume a single directory entry */ +*netname = strdup(entry-d_name); +ret = 0; +break; + } + + closedir(dir); + +out: + VIR_FREE(pcidev_sysfs_net_path); + + return ret; +} diff --git a/src/util/pci.h b/src/util/pci.h index 367881e..0cdc6ec 100644 --- a/src/util/pci.h +++ b/src/util/pci.h @@ -92,10 +92,25 @@ int pciGetVirtualFunctionsLinux(const char *sysfs_path, struct pci_config_address ***virtual_functions, unsigned int *num_virtual_functions); +# define pciDeviceIsVirtualFunction(v) pciDeviceIsVirtualFunctionLinux(v) +int pciDeviceIsVirtualFunctionLinux(const char *vf_sysfs_device_link); + +# define pciGetVirtualFunctionIndex(p,v,i) \ + pciGetVirtualFunctionIndexLinux(p,v,i) +int pciGetVirtualFunctionIndexLinux(const char *pf_sysfs_device_link, +const char *vf_sysfs_device_link, +int *vf_index); + +# define pciDeviceNetName(d,n)
Re: [libvirt] [PATCH 3/4 v2] interface: Add functions to get sriov PF/VF relationship of a net interface
On 8/15/11 4:13 AM, Stefan Berger stef...@linux.vnet.ibm.com wrote: On 08/12/2011 07:14 PM, Roopa Prabhu wrote: From: Roopa Prabhuropra...@cisco.com This patch adds the following functions to get PF/VF relationship of an SRIOV network interface: ifaceIsVirtualFunction: Function to check if a network interface is a SRIOV VF ifaceGetVirtualFunctionIndex: Function to get VF index if a network interface is a SRIOV VF ifaceGetPhysicalFunction: Function to get the PF net interface name of a SRIOV VF net interface Signed-off-by: Roopa Prabhuropra...@cisco.com Signed-off-by: Christian Benvenutibe...@cisco.com Signed-off-by: David Wangdwa...@cisco.com --- src/util/interface.c | 150 ++ src/util/interface.h |9 +++ 2 files changed, 159 insertions(+), 0 deletions(-) diff --git a/src/util/interface.c b/src/util/interface.c index 8b4522b..ec00606 100644 --- a/src/util/interface.c +++ b/src/util/interface.c @@ -45,6 +45,8 @@ #include virfile.h #include memory.h #include netlink.h +#include pci.h +#include logging.h #define VIR_FROM_THIS VIR_FROM_NET @@ -1196,3 +1198,151 @@ ifaceRestoreMacAddress(const char *linkdev, return rc; } + +#if __linux__ +static int +ifaceSysfsFile(char **pf_sysfs_device_link, const char *ifname, + const char *file) +{ + +if (virAsprintf(pf_sysfs_device_link, NET_SYSFS %s/%s, +ifname, file) 0) { +virReportOOMError(); +return -1; +} + +return 0; +} + +static int +ifaceSysfsDeviceFile(char **pf_sysfs_device_link, const char *ifname, + const char *file) +{ + +if (virAsprintf(pf_sysfs_device_link, NET_SYSFS %s/device/%s, +ifname, file) 0) { +virReportOOMError(); +return -1; +} + +return 0; +} + +/** + * ifaceIsVirtualFunction + * + * @ifname : name of the interface + * + * Checks if an interface is a SRIOV virtual function. + * + * Returns 1 if interface is SRIOV virtual function, 0 if not and -1 if error + * + */ +int +ifaceIsVirtualFunction(const char *ifname) +{ +char *if_sysfs_device_link = NULL; +int ret = -1; + +if (ifaceSysfsFile(if_sysfs_device_link, ifname, device)) +return ret; + +ret = pciDeviceIsVirtualFunction(if_sysfs_device_link); + +VIR_FREE(if_sysfs_device_link); + +return ret; +} + +/** + * ifaceGetVirtualFunctionIndex + * + * @pfname : name of the physical function interface name + * @vfname : name of the virtual function interface name + * @vf_index : Pointer to int. Contains vf index of interface upon successful + * return + * + * Returns 0 on success, -1 on failure + * + */ +int +ifaceGetVirtualFunctionIndex(const char *pfname, const char *vfname, + int *vf_index) +{ +char *pf_sysfs_device_link = NULL, *vf_sysfs_device_link = NULL; +int ret = -1; + +if (ifaceSysfsFile(pf_sysfs_device_link, pfname, device)) +return ret; + +if (ifaceSysfsFile(vf_sysfs_device_link, vfname, device)) { +VIR_FREE(pf_sysfs_device_link); +return ret; +} + +ret = pciGetVirtualFunctionIndex(pf_sysfs_device_link, + vf_sysfs_device_link, + vf_index); + +VIR_FREE(pf_sysfs_device_link); +VIR_FREE(vf_sysfs_device_link); + +return ret; +} + +/** + * ifaceGetPhysicalFunction + * + * @ifname : name of the physical function interface name + * @pfname : Contains sriov physical function for interface ifname + * upon successful return + * + * Returns 0 on success, -1 on failure + * + */ +int +ifaceGetPhysicalFunction(const char *ifname, char **pfname) +{ +char *physfn_sysfs_path = NULL; +int ret = -1; + +if (ifaceSysfsDeviceFile(physfn_sysfs_path, ifname, physfn)) +return ret; + +ret = pciDeviceNetName(physfn_sysfs_path, pfname); + +VIR_FREE(physfn_sysfs_path); + +return ret; +} +#else + +int +ifaceIsVirtualFunction(const char *ifname) +{ +ifaceError(VIR_ERR_INTERNAL_ERROR, %s, + _(ifaceIsVirtualFunction is not supported on non-linux + platforms)); +return -ENOSYS; Since above functions are described to return -1 on error, also do this in these ones here. Will do. Thanks. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] Ensure client streams are closed when marking a client for close
On 08/15/2011 11:15 AM, Daniel P. Berrange wrote: From: Daniel P. Berrangeberra...@redhat.com NB, previous patch was borked due to bad rebase Every active stream results in a reference being held on the virNetServerClientPtr object. This meant that if a client quit with any streams active, although all I/O was stopped the virNetServerClientPtr object would leak. This causes libvirtd to leak any file handles associated with open streams when a client quit To fix this, when we call virNetServerClientClose there is a callback invoked which lets the daemon release the streams and thus the extra references * daemon/remote.c: Add a hook to close all streams * daemon/stream.c, daemon/stream.h: Add API for releasing all streams * src/rpc/virnetserverclient.c, src/rpc/virnetserverclient.h: Allow registration of a hook to trigger when closing client --- daemon/remote.c | 11 ++- daemon/stream.c | 38 -- daemon/stream.h |3 +++ src/rpc/virnetserverclient.c | 21 + src/rpc/virnetserverclient.h |5 + 5 files changed, 71 insertions(+), 7 deletions(-) +void +daemonRemoveAllClientStreams(daemonClientStream *stream) +{ +daemonClientStream *tmp; + +VIR_DEBUG(stream=%p, stream); + +while (stream) { +tmp = stream-next; + +if (!stream-closed) { +virStreamEventRemoveCallback(stream-st); +virStreamAbort(stream-st); +} + +daemonFreeClientStream(NULL, stream); + +VIR_DEBUG(next stream=%p, stream); +stream = tmp; Is that the right VIR_DEBUG message, or should it be: VIR_DEBUG(next stream=%p, tmp); ACK with that fixed. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Ensure stream is aborted when exiting console
On 08/15/2011 11:12 AM, Daniel P. Berrange wrote: From: Daniel P. Berrangeberra...@redhat.com After running 'virsh console' in interactive mode, there was a missing call to virStreamAbort, which meant the server kept the stream resources open * tools/console.c: Abort stream when exiting --- tools/console.c |6 -- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/tools/console.c b/tools/console.c index 171ebc9..82ef537 100644 --- a/tools/console.c +++ b/tools/console.c @@ -90,9 +90,11 @@ static void virConsoleShutdown(virConsolePtr con) { con-quit = true; -virStreamEventRemoveCallback(con-st); -if (con-st) +if (con-st) { +virStreamEventRemoveCallback(con-st); +virStreamAbort(con-st); virStreamFree(con-st); +} ACK. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix domxml-from-native xen-sxpr for domain/clock/ @offset='localtime'
Hello Eric, Am Freitag 12 August 2011 23:40:34 schrieb Eric Blake: On 04/27/2011 07:20 AM, Philipp Hahn wrote: At least Xen-3.4.3 translates the /vm/localtime SXPR value to /domain/platform/localtime and /domain/image/{linux,hvm}/localtime when the domain is defined. When reading back that information libvirt still tries to read /domain/localtime, which now isn't found anymore. Instead domxml-from-native should read back /domain/platform/localtime. This is tracked athttps://forge.univention.org/bugzilla/show_bug.cgi?id=22321 Looking through old threads - is this patch still needed? I think that patch was wrong, but there was/is still a bug for Xen-PV domains; see other email for my patch. Sincerely Philipp Hahn -- Philipp Hahn Open Source Software Engineer h...@univention.de Univention GmbHLinux for Your Businessfon: +49 421 22 232- 0 Mary-Somerville-Str.1 D-28359 Bremen fax: +49 421 22 232-99 http://www.univention.de/ signature.asc Description: This is a digitally signed message part. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Ensure stream is aborted when exiting console
On Mon, Aug 15, 2011 at 10:12:54AM -0700, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com After running 'virsh console' in interactive mode, there was a missing call to virStreamAbort, which meant the server kept the stream resources open The combination of this patch and the other stream related patch makes the problem I was seeing of garbled console following multiple console connect/disconnects go away, but another problem remains in this area. After connecting to the console in interactive virsh and disconnecting, I often see all subsequent commands fail: virsh # dominfo foo error: failed to get domain 'foo' error: An error occurred, but the cause is unknown virsh # dominfo foo error: failed to get domain 'foo' error: no call waiting for reply with prog 536903814 vers 1 serial 300 virsh # define bar.xml error: Failed to define domain from bar.xml error: no call waiting for reply with prog 536903814 vers 1 serial 301 Closing and reopening the interactive virsh session makes the problem go away. Powering down the guest while the console is connected has made the problem 100% reproducible in the 4 or 5 tries I've given it. Dave * tools/console.c: Abort stream when exiting --- tools/console.c |6 -- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/tools/console.c b/tools/console.c index 171ebc9..82ef537 100644 --- a/tools/console.c +++ b/tools/console.c @@ -90,9 +90,11 @@ static void virConsoleShutdown(virConsolePtr con) { con-quit = true; -virStreamEventRemoveCallback(con-st); -if (con-st) +if (con-st) { +virStreamEventRemoveCallback(con-st); +virStreamAbort(con-st); virStreamFree(con-st); +} if (con-stdinWatch != -1) virEventRemoveHandle(con-stdinWatch); if (con-stdoutWatch != -1) -- 1.7.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] maint: ignore generated file
* .gitignore: Ignore qemu-sanlock.conf. --- I backported the latest sanlock-devel and netcf-devel from rawhide back to F14, so that I can test more code without upgrading distros, and noticed this file in 'git status'. Pushing under the trivial rule. .gitignore |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/.gitignore b/.gitignore index 60220ad..e8b2dbf 100644 --- a/.gitignore +++ b/.gitignore @@ -54,6 +54,7 @@ /proxy/ /python/generator.py.stamp /src/libvirt_iohelper +/src/locking/qemu-sanlock.conf /src/remote/*_client_bodies.h /src/remote/*_protocol.[ch] /src/rpc/virnetprotocol.[ch] -- 1.7.4.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] RFC (V2) New virDomainBlockPull API family to libvirt
On Mon, Aug 15, 2011 at 1:36 PM, Adam Litke a...@us.ibm.com wrote: On 08/14/2011 11:40 PM, Zhi Yong Wu wrote: HI, Deniel and Adam. Have the patchset been merged into libvirt upstream? Yes they have. However, the functionality is still missing from qemu. The two communities have agreed upon the interface and semantics, but work continues on the qemu implementation. Let me know if you would like a link to some qemu patches that support this functionality for qed images. I also have a series to put these commands into QEMU without any image format support. They just return NotSupported but it puts the commands into QEMU so we can run the libvirt commands against them. Will send those patches to qemu-devel soon. Stefan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 02/26] snapshot: better event when reverting qemu to paused snapshot
When reverting a running domain to a paused snapshot, the event that fires should mention that the domain is suspended. * include/libvirt/libvirt.h.in (VIR_DOMAIN_EVENT_SUSPENDED_FROM_SNAPSHOT): New sub-event. * src/qemu/qemu_driver.c (qemuDomainRevertToSnapshot): Use it. --- include/libvirt/libvirt.h.in |3 ++- src/qemu/qemu_driver.c | 10 ++ 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index aa29fb6..a625479 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -2010,7 +2010,7 @@ typedef enum { VIR_DOMAIN_EVENT_STARTED_BOOTED = 0, /* Normal startup from boot */ VIR_DOMAIN_EVENT_STARTED_MIGRATED = 1, /* Incoming migration from another host */ VIR_DOMAIN_EVENT_STARTED_RESTORED = 2, /* Restored from a state file */ -VIR_DOMAIN_EVENT_STARTED_FROM_SNAPSHOT = 3, /* Restored from snapshot */ +VIR_DOMAIN_EVENT_STARTED_FROM_SNAPSHOT = 3, /* Restored from running snapshot */ } virDomainEventStartedDetailType; /** @@ -2023,6 +2023,7 @@ typedef enum { VIR_DOMAIN_EVENT_SUSPENDED_MIGRATED = 1, /* Suspended for offline migration */ VIR_DOMAIN_EVENT_SUSPENDED_IOERROR = 2, /* Suspended due to a disk I/O error */ VIR_DOMAIN_EVENT_SUSPENDED_WATCHDOG = 3, /* Suspended due to a watchdog firing */ +VIR_DOMAIN_EVENT_SUSPENDED_FROM_SNAPSHOT = 4, /* Restored from paused snapshot */ } virDomainEventSuspendedDetailType; /** diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 421a98e..78d99f4 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8804,14 +8804,16 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, QEMU_ASYNC_JOB_NONE); if (rc 0) goto endjob; +event = virDomainEventNewFromObj(vm, + VIR_DOMAIN_EVENT_SUSPENDED, + VIR_DOMAIN_EVENT_SUSPENDED_FROM_SNAPSHOT); } else { virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, VIR_DOMAIN_RUNNING_FROM_SNAPSHOT); +event = virDomainEventNewFromObj(vm, + VIR_DOMAIN_EVENT_STARTED, + VIR_DOMAIN_EVENT_STARTED_FROM_SNAPSHOT); } - -event = virDomainEventNewFromObj(vm, - VIR_DOMAIN_EVENT_STARTED, - VIR_DOMAIN_EVENT_STARTED_FROM_SNAPSHOT); } else { /* qemu is a little funny with running guests and the restoration * of snapshots. If the snapshot was taken online, -- 1.7.4.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 00/26] revised round of snapshot patches
Many, but not all, of these patches have been previously submitted. I've rebased the series, and added in some additional patches throughout. It still isn't complete, and could probably use more testing after about patch 20 or so, but I'd like to get reviews on some of these to start getting these off the queue. Eric Blake (26): virsh: concatenate qemu-monitor-command arguments snapshot: better event when reverting qemu to paused snapshot snapshot: improve reverting to qemu paused snapshots snapshot: properly revert qemu to offline snapshots snapshot: one less point of failure in qemu snapshot: only pass snapshot to qemu command line when reverting snapshot: track current snapshot across restarts snapshot: allow deletion of just snapshot metadata snapshot: avoid crash when deleting qemu snapshots snapshot: simplify acting on just children snapshot: let qemu discard only snapshot metadata snapshot: identify which snapshots have metadata snapshot: identify qemu snapshot roots snapshot: prevent stranding snapshot data on domain destruction snapshot: refactor some qemu code snapshot: cache qemu-img location snapshot: support new undefine flags in qemu snapshot: teach virsh about new undefine flags snapshot: prevent migration from stranding snapshot data snapshot: refactor domain xml output snapshot: allow full domain xml in snapshot snapshot: update rng to support full domain in xml snapshot: store qemu domain details in xml snapshot: add 2 attributes to domain xml for disks snapshot: reject transient disks where code is not ready snapshot: wire up new qemu monitor command docs/formatdomain.html.in | 40 +- docs/formatsnapshot.html.in| 45 +- docs/schemas/Makefile.am |1 + docs/schemas/domain.rng| 2555 +--- docs/schemas/{domain.rng = domaincommon.rng} | 25 +- docs/schemas/domainsnapshot.rng| 19 +- include/libvirt/libvirt.h.in | 40 +- src/conf/domain_conf.c | 503 -- src/conf/domain_conf.h | 41 +- src/esx/esx_driver.c | 35 +- src/libvirt.c | 91 +- src/libvirt_private.syms |4 + src/libxl/libxl_conf.c |5 + src/qemu/qemu_command.c| 12 +- src/qemu/qemu_conf.h |1 + src/qemu/qemu_driver.c | 839 +--- src/qemu/qemu_migration.c |2 +- src/qemu/qemu_monitor.c| 24 + src/qemu/qemu_monitor.h|4 + src/qemu/qemu_monitor_json.c | 33 + src/qemu/qemu_monitor_json.h |4 + src/qemu/qemu_monitor_text.c | 40 + src/qemu/qemu_monitor_text.h |4 + src/qemu/qemu_process.c| 10 +- src/qemu/qemu_process.h|1 + src/vbox/vbox_tmpl.c | 40 +- src/xenxs/xen_sxpr.c |5 + src/xenxs/xen_xm.c |5 + tests/domainsnapshotxml2xmlout/full_domain.xml | 35 + tools/virsh.c | 419 - tools/virsh.pod| 42 +- 31 files changed, 1780 insertions(+), 3144 deletions(-) copy docs/schemas/{domain.rng = domaincommon.rng} (99%) create mode 100644 tests/domainsnapshotxml2xmlout/full_domain.xml -- 1.7.4.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 08/26] snapshot: allow deletion of just snapshot metadata
A future patch will make it impossible to remove a domain if it would leave behind any libvirt-tracked metadata about snapshots, since stale metadata interferes with a new domain by the same name. But requiring snaphot contents to be deleted before removing a domain is harsh; with qemu, qemu-img can still make use of the contents after the libvirt domain is gone. Therefore, we need an option to get rid of libvirt tracking information, but not the actual contents. For hypervisors that do not track any metadata in libvirt, the implementation is trivial; all remaining hypervisors (really, just qemu) will be dealt with separately. * include/libvirt/libvirt.h.in (VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY): New flag. * src/libvirt.c (virDomainSnapshotDelete): Document it. * src/esx/esx_driver.c (esxDomainSnapshotDelete): Trivially supported when there is no libvirt metadata. * src/vbox/vbox_tmpl.c (vboxDomainSnapshotDelete): Likewise. --- include/libvirt/libvirt.h.in |3 ++- src/esx/esx_driver.c | 10 +- src/libvirt.c| 10 +++--- src/vbox/vbox_tmpl.c | 10 +- 4 files changed, 27 insertions(+), 6 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index a625479..eae0a10 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -2579,7 +2579,8 @@ int virDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, /* Delete a snapshot */ typedef enum { -VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN = (1 0), +VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN = (1 0), /* Also delete children */ +VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY = (1 1), /* Delete just metadata */ } virDomainSnapshotDeleteFlags; int virDomainSnapshotDelete(virDomainSnapshotPtr snapshot, diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index c097651..beeafbd 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -4543,7 +4543,8 @@ esxDomainSnapshotDelete(virDomainSnapshotPtr snapshot, unsigned int flags) esxVI_TaskInfoState taskInfoState; char *taskInfoErrorMessage = NULL; -virCheckFlags(VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN, -1); +virCheckFlags(VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN | + VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY, -1); if (esxVI_EnsureSession(priv-primary) 0) { return -1; @@ -4561,6 +4562,13 @@ esxDomainSnapshotDelete(virDomainSnapshotPtr snapshot, unsigned int flags) goto cleanup; } +/* ESX snapshots do not require any libvirt metadata, making this + * flag trivial once we know we have a valid snapshot. */ +if (flags VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY) { +result = 0; +goto cleanup; +} + if (esxVI_RemoveSnapshot_Task(priv-primary, snapshotTree-snapshot, removeChildren, task) 0 || esxVI_WaitForTaskCompletion(priv-primary, task, snapshot-domain-uuid, diff --git a/src/libvirt.c b/src/libvirt.c index c8af3e1..8ee9e96 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -15792,14 +15792,18 @@ error: /** * virDomainSnapshotDelete: * @snapshot: a domain snapshot object - * @flags: flag parameters + * @flags: bitwise-or of supported virDomainSnapshotDeleteFlags * * Delete the snapshot. * * If @flags is 0, then just this snapshot is deleted, and changes from * this snapshot are automatically merged into children snapshots. If - * flags is VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN, then this snapshot - * and any children snapshots are deleted. + * @flags includes VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN, then this snapshot + * and any children snapshots are deleted. If @flags includes + * VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY, then any snapshot metadata + * tracked by libvirt is removed while keeping the snapshot contents + * intact; if a hypervisor does not require any libvirt metadata to + * track snapshots, then this flag is silently ignored. * * Returns 0 if the snapshot was successfully deleted, -1 on error. */ diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index 822e899..8de2bae 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -6361,7 +6361,8 @@ vboxDomainSnapshotDelete(virDomainSnapshotPtr snapshot, PRUint32 state; nsresult rc; -virCheckFlags(VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN, -1); +virCheckFlags(VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN | + VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY, -1); vboxIIDFromUUID(domiid, dom-uuid); rc = VBOX_OBJECT_GET_MACHINE(domiid.value, machine); @@ -6382,6 +6383,13 @@ vboxDomainSnapshotDelete(virDomainSnapshotPtr snapshot, goto cleanup; } +/* VBOX snapshots do not require any libvirt metadata, making this + * flag trivial once we know we have a valid snapshot. */ +if (flags VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY) { +ret = 0; +goto cleanup; +} + if (state = MachineState_FirstOnline
[libvirt] [PATCHv2 03/26] snapshot: improve reverting to qemu paused snapshots
If you take a checkpoint snapshot of a running domain, then pause qemu, then restore the snapshot, the result should be a running domain, but the code was leaving things paused. Furthermore, if you take a checkpoint of a paused domain, then run, then restore, there was a brief but non-deterministic window of time where the domain was running rather than paused. Fix both of these discrepancies by always pausing before restoring. Also, check that the VM is active every time lock is dropped between two monitor calls. * src/qemu/qemu_driver.c (qemuDomainRevertToSnapshot): Always pause before reversion. --- src/qemu/qemu_driver.c | 61 +-- 1 files changed, 43 insertions(+), 18 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 78d99f4..aa53c63 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8772,44 +8772,69 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, if (snap-def-state == VIR_DOMAIN_RUNNING || snap-def-state == VIR_DOMAIN_PAUSED) { - +/* When using the loadvm monitor command, qemu does not know + * whether to pause or run the reverted domain, and just stays + * in the same state as before the monitor command, whether + * that is paused or running. We always pause before loadvm, + * to have finer control. */ if (virDomainObjIsActive(vm)) { priv = vm-privateData; +if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) { +if (qemuProcessStopCPUs(driver, vm, +VIR_DOMAIN_PAUSED_FROM_SNAPSHOT, +QEMU_ASYNC_JOB_NONE) 0) +goto endjob; +/* Create an event now in case the restore fails, so + * that user will be alerted that they are now paused. + * If restore later succeeds to a running state, we + * replace this event with another. */ +event = virDomainEventNewFromObj(vm, + VIR_DOMAIN_EVENT_SUSPENDED, + VIR_DOMAIN_EVENT_STARTED_FROM_SNAPSHOT); +if (!virDomainObjIsActive(vm)) { +qemuReportError(VIR_ERR_INTERNAL_ERROR, %s, +_(guest unexpectedly quit)); +goto endjob; +} +} qemuDomainObjEnterMonitorWithDriver(driver, vm); rc = qemuMonitorLoadSnapshot(priv-mon, snap-def-name); qemuDomainObjExitMonitorWithDriver(driver, vm); -if (rc 0) +if (rc 0) { +/* XXX resume domain if it was running before the + * failed loadvm attempt? */ goto endjob; +} } else { if (qemuDomainSnapshotSetCurrentActive(vm, driver-snapshotDir) 0) goto endjob; rc = qemuProcessStart(snapshot-domain-conn, driver, vm, NULL, - false, false, -1, NULL, VIR_VM_OP_CREATE); + true, false, -1, NULL, VIR_VM_OP_CREATE); virDomainAuditStart(vm, from-snapshot, rc = 0); +if (rc 0) +goto endjob; if (qemuDomainSnapshotSetCurrentInactive(vm, driver-snapshotDir) 0) goto endjob; -if (rc 0) -goto endjob; } +/* Touch up domain state. */ if (snap-def-state == VIR_DOMAIN_PAUSED) { -/* qemu unconditionally starts the domain running again after - * loadvm, so let's pause it to keep consistency - * XXX we should have used qemuProcessStart's start_paused instead - */ -rc = qemuProcessStopCPUs(driver, vm, - VIR_DOMAIN_PAUSED_FROM_SNAPSHOT, - QEMU_ASYNC_JOB_NONE); +virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, + VIR_DOMAIN_PAUSED_FROM_SNAPSHOT); +} else { +if (!virDomainObjIsActive(vm)) { +qemuReportError(VIR_ERR_INTERNAL_ERROR, %s, +_(guest unexpectedly quit)); +goto endjob; +} +rc = qemuProcessStartCPUs(driver, vm, snapshot-domain-conn, + VIR_DOMAIN_RUNNING_FROM_SNAPSHOT, + QEMU_ASYNC_JOB_NONE); if (rc 0) goto endjob; -event = virDomainEventNewFromObj(vm, - VIR_DOMAIN_EVENT_SUSPENDED, - VIR_DOMAIN_EVENT_SUSPENDED_FROM_SNAPSHOT); -} else { -
[libvirt] [PATCHv2 01/26] virsh: concatenate qemu-monitor-command arguments
Call me lazy, but: virsh qemu-monitor-command dom --hmp info status is nicer than: virsh qemu-monitor-command dom --hmp 'info status' * tools/virsh.c (cmdQemuMonitorCommand): Allow multiple arguments, for convenience. --- tools/virsh.c | 19 +++ tools/virsh.pod |6 -- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 51ba0a8..c094911 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -12476,8 +12476,8 @@ static const vshCmdInfo info_qemu_monitor_command[] = { static const vshCmdOptDef opts_qemu_monitor_command[] = { {domain, VSH_OT_DATA, VSH_OFLAG_REQ, N_(domain name, id or uuid)}, -{cmd, VSH_OT_DATA, VSH_OFLAG_REQ, N_(command)}, {hmp, VSH_OT_BOOL, 0, N_(command is in human monitor protocol)}, +{cmd, VSH_OT_ARGV, VSH_OFLAG_REQ, N_(command)}, {NULL, 0, 0, NULL} }; @@ -12486,9 +12486,12 @@ cmdQemuMonitorCommand(vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom = NULL; bool ret = false; -const char *monitor_cmd = NULL; +char *monitor_cmd = NULL; char *result = NULL; unsigned int flags = 0; +const vshCmdOpt *opt = NULL; +virBuffer buf = VIR_BUFFER_INITIALIZER; +bool pad = false; if (!vshConnectionUsability(ctl, ctl-conn)) goto cleanup; @@ -12497,10 +12500,17 @@ cmdQemuMonitorCommand(vshControl *ctl, const vshCmd *cmd) if (dom == NULL) goto cleanup; -if (vshCommandOptString(cmd, cmd, monitor_cmd) = 0) { -vshError(ctl, %s, _(missing monitor command)); +while ((opt = vshCommandOptArgv(cmd, opt))) { +if (pad) +virBufferAddChar(buf, ' '); +pad = true; +virBufferAdd(buf, opt-data, -1); +} +if (virBufferError(buf)) { +vshPrint(ctl, %s, _(Failed to collect command)); goto cleanup; } +monitor_cmd = virBufferContentAndReset(buf); if (vshCommandOptBool(cmd, hmp)) flags |= VIR_DOMAIN_QEMU_MONITOR_COMMAND_HMP; @@ -12514,6 +12524,7 @@ cmdQemuMonitorCommand(vshControl *ctl, const vshCmd *cmd) cleanup: VIR_FREE(result); +VIR_FREE(monitor_cmd); if (dom) virDomainFree(dom); diff --git a/tools/virsh.pod b/tools/virsh.pod index 1893c23..11a13fd 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1689,13 +1689,15 @@ attaching to an externally launched QEMU process. There may be issues with the guest ABI changing upon migration, and hotunplug may not work. -=item Bqemu-monitor-command Idomain Icommand [I--hmp] +=item Bqemu-monitor-command Idomain [I--hmp] Icommand... Send an arbitrary monitor command Icommand to domain Idomain through the qemu monitor. The results of the command will be printed on stdout. If I--hmp is passed, the command is considered to be a human monitor command and libvirt will automatically convert it into QMP if needed. In that case -the result will also be converted back from QMP. +the result will also be converted back from QMP. If more than one argument +is provided for Icommand, they are concatenated with a space in between +before passing the single command to the monitor. =back -- 1.7.4.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 14/26] snapshot: prevent stranding snapshot data on domain destruction
Just as leaving managed save metadata behind can cause problems when creating a new domain that happens to collide with the name of the just-deleted domain, the same is true of leaving any snapshot metadata behind. For safety sake, extend the semantic change of commit b26a9fa9 to also cover snapshot metadata as a reason to reject losing the last reference to a domain (undefine on an inactive, or shutdown/destroy on a transient). The caller must first take care of snapshots, possible via the existing virDomainSnapshotDelete. This also documents some new flags that hypervisors can choose to support to shortcuts taking care of the metadata as part of the shutdown process; however, nontrivial driver support for these flags will be deferred to future patches. Note that ESX and VBox can never be transient; therefore, they do not have to affect shutdown/destroy (the persistent domain still remains); likewise they never store snapshot metadata, so one of the two flags is trivial. The bulk of the nontrivial work remaining is thus in the qemu driver. * include/libvirt/libvirt.h.in (VIR_DOMAIN_UNDEFINE_SNAPSHOTS) (VIR_DOMAIN_DESTROY_SNAPSHOTS): New flags. * src/libvirt.c (virDomainUndefine, virDomainUndefineFlags) (virDomainDestroy, virDomainDestroyFlags, virDomainShutdown): Document new limitations and flags. * src/esx/esx_driver.c (esxDomainUndefineFlags): Enforce the limitations. * src/vbox/vbox_tmpl.c (vboxDomainUndefineFlags): Likewise. * src/qemu/qemu_driver.c (qemuDomainUndefineFlags) (qemuDomainShutdown, qemuDomainDestroyFlags): Likewise. --- include/libvirt/libvirt.h.in | 25 ++--- src/esx/esx_driver.c | 11 - src/libvirt.c| 47 +++-- src/qemu/qemu_driver.c | 26 +++ src/vbox/vbox_tmpl.c | 11 - 5 files changed, 105 insertions(+), 15 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 20fdbdf..36f1b34 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -919,10 +919,20 @@ virConnectPtr virDomainGetConnect (virDomainPtr domain); * Domain creation and destruction */ -/* - * typedef enum { - * } virDomainDestroyFlagsValues; + +/* Counterparts to virDomainUndefineFlagsValues, but note that running + * domains have no managed save data, so no flag is provided for that. */ +typedef enum { +/* VIR_DOMAIN_DESTROY_MANAGED_SAVE = (1 0), */ /* Reserved */ +VIR_DOMAIN_DESTROY_SNAPSHOTS_METADATA = (1 1), /* If last use of domain, + then also remove any + snapshot metadata */ +VIR_DOMAIN_DESTROY_SNAPSHOTS_FULL = (1 2), /* If last use of domain, + then also remove any + snapshot data */ +} virDomainDestroyFlagsValues; + virDomainPtrvirDomainCreateXML (virConnectPtr conn, const char *xmlDesc, unsigned int flags); @@ -1240,7 +1250,14 @@ virDomainPtrvirDomainDefineXML (virConnectPtr conn, int virDomainUndefine (virDomainPtr domain); typedef enum { -VIR_DOMAIN_UNDEFINE_MANAGED_SAVE = (1 0), +VIR_DOMAIN_UNDEFINE_MANAGED_SAVE = (1 0), /* Also remove any + managed save */ +VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA = (1 1), /* If last use of domain, + then also remove any + snapshot metadata */ +VIR_DOMAIN_UNDEFINE_SNAPSHOTS_FULL = (1 2), /* If last use of domain, + then also remove any + snapshot data */ /* Future undefine control flags should come here. */ } virDomainUndefineFlagsValues; diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index dbc7694..90f55c3 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -1950,7 +1950,9 @@ esxDomainDestroyFlags(virDomainPtr domain, esxVI_TaskInfoState taskInfoState; char *taskInfoErrorMessage = NULL; -virCheckFlags(0, -1); +/* No transient domains, so these flags are trivially ignored. */ +virCheckFlags(VIR_DOMAIN_DESTROY_SNAPSHOTS_METADATA | + VIR_DOMAIN_DESTROY_SNAPSHOTS_FULL, -1); if (priv-vCenter != NULL) { ctx = priv-vCenter; @@ -3309,7 +3311,9 @@ esxDomainUndefineFlags(virDomainPtr domain, esxVI_String *propertyNameList = NULL; esxVI_VirtualMachinePowerState powerState; -virCheckFlags(0, -1); +/* No managed save, so we explicitly reject + *
[libvirt] [PATCHv2 05/26] snapshot: one less point of failure in qemu
https://bugzilla.redhat.com/show_bug.cgi?id=727709 mentions that if qemu fails to create the snapshot (such as what happens on Fedora 15 qemu, which has qmp but where savevm is only in hmp, and where libvirt is old enough to not try the hmp fallback), then 'virsh snapshot-list dom' will show a garbage snapshot entry, and the libvirt internal directory for storing snapshot metadata. This fixes the fallout bug of polluting the snapshot-list with garbage on failure (the root cause of the F15 bug of not having fallback to hmp has already been fixed in newer libvirt releases). * src/qemu/qemu_driver.c (qemuDomainSnapshotCreateXML): Allocate memory before making snapshot, and cleanup on failure. --- src/qemu/qemu_driver.c | 37 +++-- 1 files changed, 19 insertions(+), 18 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 989d21b..b83e1f0 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8470,7 +8470,7 @@ static virDomainSnapshotPtr qemuDomainSnapshotCreateXML(virDomainPtr domain, virDomainSnapshotObjPtr snap = NULL; virDomainSnapshotPtr snapshot = NULL; char uuidstr[VIR_UUID_STRING_BUFLEN]; -virDomainSnapshotDefPtr def; +virDomainSnapshotDefPtr def = NULL; virCheckFlags(0, NULL); @@ -8500,6 +8500,14 @@ static virDomainSnapshotPtr qemuDomainSnapshotCreateXML(virDomainPtr domain, goto cleanup; snap-def-state = virDomainObjGetState(vm, NULL); +if (vm-current_snapshot) { +def-parent = strdup(vm-current_snapshot-def-name); +if (def-parent == NULL) { +virReportOOMError(); +goto cleanup; +} +vm-current_snapshot = NULL; +} /* actually do the snapshot */ if (!virDomainObjIsActive(vm)) { @@ -8511,32 +8519,25 @@ static virDomainSnapshotPtr qemuDomainSnapshotCreateXML(virDomainPtr domain, goto cleanup; } -/* FIXME: if we fail after this point, there's not a whole lot we can +/* If we fail after this point, there's not a whole lot we can * do; we've successfully taken the snapshot, and we are now running * on it, so we have to go forward the best we can */ - -if (vm-current_snapshot) { -def-parent = strdup(vm-current_snapshot-def-name); -if (def-parent == NULL) { -virReportOOMError(); -goto cleanup; -} -} - -/* Now we set the new current_snapshot for the domain */ -vm-current_snapshot = snap; - -if (qemuDomainSnapshotWriteMetadata(vm, vm-current_snapshot, -driver-snapshotDir) 0) -/* qemuDomainSnapshotWriteMetadata set the error */ +if (qemuDomainSnapshotWriteMetadata(vm, snap, driver-snapshotDir) 0) goto cleanup; snapshot = virGetDomainSnapshot(domain, snap-def-name); cleanup: -if (vm) +if (vm) { +if (snapshot) +vm-current_snapshot = snap; +else if (snap) +virDomainSnapshotObjListRemove(vm-snapshots, snap); +else +virDomainSnapshotDefFree(def); virDomainObjUnlock(vm); +} qemuDriverUnlock(driver); return snapshot; } -- 1.7.4.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 12/26] snapshot: identify which snapshots have metadata
To make it easier to know when undefine will fail because of existing snapshot metadata, we need to know how many snapshots have metadata. Also, it is handy to filter the list of snapshots to just those that have no parents; document that flag now, but implement it in later patches. * include/libvirt/libvirt.h.in (VIR_DOMAIN_SNAPSHOT_LIST_ROOTS) (VIR_DOMAIN_SNAPSHOT_LIST_METADATA): New flags. * src/libvirt.c (virDomainSnapshotNum) (virDomainSnapshotListNames): Document them. * src/esx/esx_driver.c (esxDomainSnapshotNum) (esxDomainSnapshotListNames): Implement trivial flag. * src/vbox/vbox_tmpl.c (vboxDomainSnapshotNum) (vboxDomainSnapshotListNames): Likewise. * src/qemu/qemu_driver.c (qemuDomainSnapshotNum) (qemuDomainSnapshotListNames): Likewise. --- include/libvirt/libvirt.h.in |9 + src/esx/esx_driver.c | 10 +++--- src/libvirt.c| 27 ++- src/qemu/qemu_driver.c |8 ++-- src/vbox/vbox_tmpl.c | 15 +-- 5 files changed, 57 insertions(+), 12 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index eae0a10..20fdbdf 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -2551,6 +2551,15 @@ virDomainSnapshotPtr virDomainSnapshotCreateXML(virDomainPtr domain, char *virDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot, unsigned int flags); +/* Flags valid for both virDomainSnapshotNum() and + * virDomainSnapshotListNames(). */ +typedef enum { +VIR_DOMAIN_SNAPSHOT_LIST_ROOTS= (1 0), /* Filter by snapshots which + have no parents */ +VIR_DOMAIN_SNAPSHOT_LIST_METADATA = (1 1), /* Filter by snapshots which + have metadata */ +} virDomainSnapshotListFlags; + /* Return the number of snapshots for this domain */ int virDomainSnapshotNum(virDomainPtr domain, unsigned int flags); diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index beeafbd..dbc7694 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -4329,12 +4329,16 @@ esxDomainSnapshotNum(virDomainPtr domain, unsigned int flags) esxPrivate *priv = domain-conn-privateData; esxVI_VirtualMachineSnapshotTree *rootSnapshotTreeList = NULL; -virCheckFlags(0, -1); +virCheckFlags(VIR_DOMAIN_SNAPSHOT_LIST_METADATA, -1); if (esxVI_EnsureSession(priv-primary) 0) { return -1; } +/* ESX snapshots do not require libvirt to maintain any metadata. */ +if (flags VIR_DOMAIN_SNAPSHOT_LIST_METADATA) +return 0; + if (esxVI_LookupRootSnapshotTreeList(priv-primary, domain-uuid, rootSnapshotTreeList) 0) { return -1; @@ -4357,14 +4361,14 @@ esxDomainSnapshotListNames(virDomainPtr domain, char **names, int nameslen, esxPrivate *priv = domain-conn-privateData; esxVI_VirtualMachineSnapshotTree *rootSnapshotTreeList = NULL; -virCheckFlags(0, -1); +virCheckFlags(VIR_DOMAIN_SNAPSHOT_LIST_METADATA, -1); if (names == NULL || nameslen 0) { ESX_ERROR(VIR_ERR_INVALID_ARG, %s, _(Invalid argument)); return -1; } -if (nameslen == 0) { +if (nameslen == 0 || (flags VIR_DOMAIN_SNAPSHOT_LIST_METADATA)) { return 0; } diff --git a/src/libvirt.c b/src/libvirt.c index 8ee9e96..30b464a 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -15526,11 +15526,19 @@ error: /** * virDomainSnapshotNum: * @domain: a domain object - * @flags: unused flag parameters; callers should pass 0 + * @flags: bitwise-or of supported virDomainSnapshotListFlags + * + * Provides the number of domain snapshots for this domain. * - * Provides the number of domain snapshots for this domain.. + * If @flags includes VIR_DOMAIN_SNAPSHOT_LIST_ROOTS, then the result is + * filtered to the number of snapshots that have no parents. * - * Returns the number of domain snapshost found or -1 in case of error. + * If @flags includes VIR_DOMAIN_SNAPSHOT_LIST_METADATA, then the result is + * the number of snapshots that also include metadata that would prevent + * the removal of the last reference to a domain; this value will either + * be 0 or the same value as if the flag were not given. + * + * Returns the number of domain snapshots found or -1 in case of error. */ int virDomainSnapshotNum(virDomainPtr domain, unsigned int flags) @@ -15566,11 +15574,20 @@ error: * @domain: a domain object * @names: array to collect the list of names of snapshots * @nameslen: size of @names - * @flags: unused flag parameters; callers should pass 0 + * @flags: bitwise-or of supported virDomainSnapshotListFlags * * Collect the list of domain snapshots for the given domain, and store * their names in @names. Caller is responsible for freeing each member - * of the array. + * of the array. The value to use for
[libvirt] [PATCHv2 11/26] snapshot: let qemu discard only snapshot metadata
Adding this was trivial compared to the previous patch for fixing qemu snapshot deletion in the first place. * src/qemu/qemu_driver.c (qemuDomainSnapshotDiscard): Add parameter. (qemuDomainSnapshotDiscardDescendant, qemuDomainSnapshotDelete): Update callers. --- src/qemu/qemu_driver.c | 78 ++- 1 files changed, 43 insertions(+), 35 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 9134fc3..568cb37 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8934,7 +8934,8 @@ cleanup: static int qemuDomainSnapshotDiscard(struct qemud_driver *driver, virDomainObjPtr vm, - virDomainSnapshotObjPtr snap) + virDomainSnapshotObjPtr snap, + bool metadata_only) { const char *qemuimgarg[] = { NULL, snapshot, -d, NULL, NULL, NULL }; char *snapFile = NULL; @@ -8943,41 +8944,43 @@ static int qemuDomainSnapshotDiscard(struct qemud_driver *driver, qemuDomainObjPrivatePtr priv; virDomainSnapshotObjPtr parentsnap = NULL; -if (!virDomainObjIsActive(vm)) { -qemuimgarg[0] = qemuFindQemuImgBinary(); -if (qemuimgarg[0] == NULL) -/* qemuFindQemuImgBinary set the error */ -goto cleanup; - -qemuimgarg[3] = snap-def-name; - -for (i = 0; i vm-def-ndisks; i++) { -/* FIXME: we also need to handle LVM here */ -if (vm-def-disks[i]-device == VIR_DOMAIN_DISK_DEVICE_DISK) { -if (!vm-def-disks[i]-driverType || -STRNEQ(vm-def-disks[i]-driverType, qcow2)) { -/* we continue on even in the face of error, since other - * disks in this VM may have this snapshot in place - */ -continue; -} - -qemuimgarg[4] = vm-def-disks[i]-src; +if (!metadata_only) { +if (!virDomainObjIsActive(vm)) { +qemuimgarg[0] = qemuFindQemuImgBinary(); +if (qemuimgarg[0] == NULL) +/* qemuFindQemuImgBinary set the error */ +goto cleanup; -if (virRun(qemuimgarg, NULL) 0) { -/* we continue on even in the face of error, since other - * disks in this VM may have this snapshot in place - */ -continue; +qemuimgarg[3] = snap-def-name; + +for (i = 0; i vm-def-ndisks; i++) { +/* FIXME: we also need to handle LVM here */ +if (vm-def-disks[i]-device == VIR_DOMAIN_DISK_DEVICE_DISK) { +if (!vm-def-disks[i]-driverType || +STRNEQ(vm-def-disks[i]-driverType, qcow2)) { +/* we continue on even in the face of error, since other + * disks in this VM may have this snapshot in place + */ +continue; +} + +qemuimgarg[4] = vm-def-disks[i]-src; + +if (virRun(qemuimgarg, NULL) 0) { +/* we continue on even in the face of error, since other + * disks in this VM may have this snapshot in place + */ +continue; +} } } +} else { +priv = vm-privateData; +qemuDomainObjEnterMonitorWithDriver(driver, vm); +/* we continue on even in the face of error */ +qemuMonitorDeleteSnapshot(priv-mon, snap-def-name); +qemuDomainObjExitMonitorWithDriver(driver, vm); } -} else { -priv = vm-privateData; -qemuDomainObjEnterMonitorWithDriver(driver, vm); -/* we continue on even in the face of error */ -qemuMonitorDeleteSnapshot(priv-mon, snap-def-name); -qemuDomainObjExitMonitorWithDriver(driver, vm); } if (virAsprintf(snapFile, %s/%s/%s.xml, driver-snapshotDir, @@ -9023,6 +9026,7 @@ cleanup: struct snap_remove { struct qemud_driver *driver; virDomainObjPtr vm; +bool metadata_only; int err; }; @@ -9035,7 +9039,8 @@ qemuDomainSnapshotDiscardDescendant(void *payload, struct snap_remove *curr = data; int err; -err = qemuDomainSnapshotDiscard(curr-driver, curr-vm, snap); +err = qemuDomainSnapshotDiscard(curr-driver, curr-vm, snap, +curr-metadata_only); if (err !curr-err) curr-err = err; } @@ -9085,8 +9090,10 @@ static int qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot, char uuidstr[VIR_UUID_STRING_BUFLEN]; struct snap_remove rem; struct snap_reparent rep; +bool metadata_only = !!(flags VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY); -
[libvirt] [PATCHv2 09/26] snapshot: avoid crash when deleting qemu snapshots
This one's nasty. Ever since we fixed virHashForEach to prevent nested hash iterations for safety reasons, virDomainSnapshotDelete with VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN has been broken for qemu: it deletes children, while leaving grandchildren intact but pointing to a no-longer-present parent. But even before then, the code would often appear to succeed to clean up grandchildren, but risked memory corruption if you have a large and deep hierarchy of snapshots. For acting on just children, a single virHashForEach is sufficient. But for acting on an entire subtree, it requires iteration; and since we declared recursion as invalid, we have to switch to a while loop. Doing this correctly requires quite a bit of overhaul, so I added a new helper function to isolate the algorithm from the actions, so that callers do not have to reinvent the iteration. * src/conf/domain_conf.h (_virDomainSnapshotDef): Add mark. (virDomainSnapshotForEachDescendant): New prototype. * src/libvirt_private.syms (domain_conf.h): Export it. * src/conf/domain_conf.c (virDomainSnapshotMarkDescendant) (virDomainSnapshotActOnDescendant) (virDomainSnapshotForEachDescendant): New functions. * src/qemu/qemu_driver.c (qemuDomainSnapshotDiscardChildren): Replace... (qemuDomainSnapshotDiscardDescenent): ...with callback that doesn't nest hash traversal. (qemuDomainSnapshotDelete): Use new function. --- src/conf/domain_conf.c | 103 ++ src/conf/domain_conf.h |8 +++- src/libvirt_private.syms |1 + src/qemu/qemu_driver.c | 35 ++-- 4 files changed, 124 insertions(+), 23 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7793a13..7704849 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11280,6 +11280,109 @@ int virDomainSnapshotHasChildren(virDomainSnapshotObjPtr snap, return children.number; } +typedef enum { +MARK_NONE, /* No relation determined yet */ +MARK_DESCENDANT, /* Descendant of target */ +MARK_OTHER, /* Not a descendant of target */ +} snapshot_mark; + +struct snapshot_mark_descendant { +const char *name; /* Parent's name on round 1, NULL on other rounds. */ +virDomainSnapshotObjListPtr snapshots; +bool marked; /* True if descendants were found in this round */ +}; + +/* To be called in a loop until no more descendants are found. + * Additionally marking known unrelated snapshots reduces the number + * of total hash searches needed. */ +static void +virDomainSnapshotMarkDescendant(void *payload, +const void *name ATTRIBUTE_UNUSED, +void *data) +{ +virDomainSnapshotObjPtr obj = payload; +struct snapshot_mark_descendant *curr = data; +virDomainSnapshotObjPtr parent = NULL; + +/* Learned on a previous pass. */ +if (obj-mark) +return; + +if (curr-name) { +/* First round can only find root nodes and direct children. */ +if (!obj-def-parent) { +obj-mark = MARK_OTHER; +} else if (STREQ(obj-def-parent, curr-name)) { +obj-mark = MARK_DESCENDANT; +curr-marked = true; +} +} else { +/* All remaining rounds propagate marks from parents to children. */ +parent = virDomainSnapshotFindByName(curr-snapshots, obj-def-parent); +if (!parent) { +VIR_WARN(snapshot hash table is inconsistent!); +obj-mark = MARK_OTHER; +return; +} +if (parent-mark) { +obj-mark = parent-mark; +if (obj-mark == MARK_DESCENDANT) +curr-marked = true; +} +} +} + +struct snapshot_act_on_descendant { +int number; +virHashIterator iter; +void *data; +}; + +static void +virDomainSnapshotActOnDescendant(void *payload, + const void *name, + void *data) +{ +virDomainSnapshotObjPtr obj = payload; +struct snapshot_act_on_descendant *curr = data; + +if (obj-mark == MARK_DESCENDANT) { +curr-number++; +(curr-iter)(payload, name, curr-data); +} +obj-mark = MARK_NONE; +} + +/* Run iter(data) on all descendants of snapshot, while ignoring all + * other entries in snapshots. Return the number of descendants + * visited. No particular ordering is guaranteed. */ +int +virDomainSnapshotForEachDescendant(virDomainSnapshotObjListPtr snapshots, + virDomainSnapshotObjPtr snapshot, + virHashIterator iter, + void *data) +{ +struct snapshot_mark_descendant mark; +struct snapshot_act_on_descendant act; + +/* virHashForEach does not support nested traversal, so we must + * instead iterate until no more snapshots get marked. We + * guarantee that on exit, all marks have been cleared again. */ +mark.name =
[libvirt] [PATCHv2 10/26] snapshot: simplify acting on just children
Similar to the last patch in isolating the filtering from the client actions, so that clients don't have to reinvent the filtering. * src/conf/domain_conf.h (virDomainSnapshotForEachChild): New prototype. * src/libvirt_private.syms (domain_conf.h): Export it. * src/conf/domain_conf.c (virDomainSnapshotActOnChild) (virDomainSnapshotForEachChild): New functions. (virDomainSnapshotCountChildren): Delete. (virDomainSnapshotHasChildren): Simplify. * src/qemu/qemu_driver.c (qemuDomainSnapshotReparentChildren) (qemuDomainSnapshotDelete): Likewise. --- src/conf/domain_conf.c | 48 - src/conf/domain_conf.h |4 +++ src/libvirt_private.syms |1 + src/qemu/qemu_driver.c | 31 ++--- 4 files changed, 54 insertions(+), 30 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7704849..f1b0aca 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11252,32 +11252,52 @@ void virDomainSnapshotObjListRemove(virDomainSnapshotObjListPtr snapshots, virHashRemoveEntry(snapshots-objs, snapshot-def-name); } -struct snapshot_has_children { -char *name; +struct snapshot_act_on_child { +char *parent; int number; +virHashIterator iter; +void *data; }; -static void virDomainSnapshotCountChildren(void *payload, - const void *name ATTRIBUTE_UNUSED, - void *data) +static void +virDomainSnapshotActOnChild(void *payload, +const void *name, +void *data) { virDomainSnapshotObjPtr obj = payload; -struct snapshot_has_children *curr = data; +struct snapshot_act_on_child *curr = data; -if (obj-def-parent STREQ(obj-def-parent, curr-name)) +if (obj-def-parent STREQ(curr-parent, obj-def-parent)) { curr-number++; +if (curr-iter) +(curr-iter)(payload, name, curr-data); +} } -int virDomainSnapshotHasChildren(virDomainSnapshotObjPtr snap, -virDomainSnapshotObjListPtr snapshots) +/* Run iter(data) on all direct children of snapshot, while ignoring all + * other entries in snapshots. Return the number of children + * visited. No particular ordering is guaranteed. */ +int +virDomainSnapshotForEachChild(virDomainSnapshotObjListPtr snapshots, + virDomainSnapshotObjPtr snapshot, + virHashIterator iter, + void *data) { -struct snapshot_has_children children; +struct snapshot_act_on_child act; -children.name = snap-def-name; -children.number = 0; -virHashForEach(snapshots-objs, virDomainSnapshotCountChildren, children); +act.parent = snapshot-def-name; +act.number = 0; +act.iter = iter; +act.data = data; +virHashForEach(snapshots-objs, virDomainSnapshotActOnChild, act); -return children.number; +return act.number; +} + +int virDomainSnapshotHasChildren(virDomainSnapshotObjPtr snap, + virDomainSnapshotObjListPtr snapshots) +{ +return virDomainSnapshotForEachChild(snapshots, snap, NULL, NULL); } typedef enum { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index d266605..5f752ec 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1343,6 +1343,10 @@ void virDomainSnapshotObjListRemove(virDomainSnapshotObjListPtr snapshots, virDomainSnapshotObjPtr snapshot); int virDomainSnapshotHasChildren(virDomainSnapshotObjPtr snap, virDomainSnapshotObjListPtr snapshots); +int virDomainSnapshotForEachChild(virDomainSnapshotObjListPtr snapshots, + virDomainSnapshotObjPtr snapshot, + virHashIterator iter, + void *data); int virDomainSnapshotForEachDescendant(virDomainSnapshotObjListPtr snapshots, virDomainSnapshotObjPtr snapshot, virHashIterator iter, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index c53b295..0d8aa99 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -384,6 +384,7 @@ virDomainSnapshotDefFormat; virDomainSnapshotDefFree; virDomainSnapshotDefParseString; virDomainSnapshotFindByName; +virDomainSnapshotForEachChild; virDomainSnapshotForEachDescendant; virDomainSnapshotHasChildren; virDomainSnapshotObjListGetNames; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f523404..9134fc3 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9042,7 +9042,7 @@ qemuDomainSnapshotDiscardDescendant(void *payload, struct snap_reparent { struct qemud_driver *driver; -virDomainSnapshotObjPtr snap; +const char *parent; virDomainObjPtr vm; int
[libvirt] [PATCHv2 13/26] snapshot: identify qemu snapshot roots
Filtering for roots is pretty easy to do. * src/conf/domain_conf.h (virDomainSnapshotObjListGetNames) (virDomainSnapshotObjListNum): Update prototype. * src/conf/domain_conf.c (virDomainSnapshotObjListCopyNames) (virDomainSnapshotObjListGetNames, virDomainSnapshotObjListCount) (virDomainSnapshotObjListNum): Support filtering. * src/qemu/qemu_driver.c (qemuDomainSnapshotNum) (qemuDomainSnapshotListNames): Update callers. --- src/conf/domain_conf.c | 34 -- src/conf/domain_conf.h |6 -- src/qemu/qemu_driver.c | 11 +++ 3 files changed, 35 insertions(+), 16 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f1b0aca..4bf3541 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11169,6 +11169,7 @@ struct virDomainSnapshotNameData { int numnames; int maxnames; char **const names; +unsigned int flags; }; static void virDomainSnapshotObjListCopyNames(void *payload, @@ -11180,6 +11181,8 @@ static void virDomainSnapshotObjListCopyNames(void *payload, if (data-oom) return; +if ((data-flags VIR_DOMAIN_SNAPSHOT_LIST_ROOTS) !obj-def-parent) +return; if (data-numnames data-maxnames) { if (!(data-names[data-numnames] = strdup(obj-def-name))) @@ -11190,9 +11193,10 @@ static void virDomainSnapshotObjListCopyNames(void *payload, } int virDomainSnapshotObjListGetNames(virDomainSnapshotObjListPtr snapshots, - char **const names, int maxnames) + char **const names, int maxnames, + unsigned int flags) { -struct virDomainSnapshotNameData data = { 0, 0, maxnames, names }; +struct virDomainSnapshotNameData data = { 0, 0, maxnames, names, flags }; int i; virHashForEach(snapshots-objs, virDomainSnapshotObjListCopyNames, data); @@ -11209,22 +11213,32 @@ cleanup: return -1; } -static void virDomainSnapshotObjListCount(void *payload ATTRIBUTE_UNUSED, +struct virDomainSnapshotNumData { +int count; +unsigned int flags; +}; + +static void virDomainSnapshotObjListCount(void *payload, const void *name ATTRIBUTE_UNUSED, - void *data) + void *opaque) { -int *count = data; +virDomainSnapshotObjPtr obj = payload; +struct virDomainSnapshotNumData *data = opaque; -(*count)++; +if ((data-flags VIR_DOMAIN_SNAPSHOT_LIST_ROOTS) +!obj-def-parent) +return; +data-count++; } -int virDomainSnapshotObjListNum(virDomainSnapshotObjListPtr snapshots) +int virDomainSnapshotObjListNum(virDomainSnapshotObjListPtr snapshots, +unsigned int flags) { -int count = 0; +struct virDomainSnapshotNumData data = { 0, flags }; -virHashForEach(snapshots-objs, virDomainSnapshotObjListCount, count); +virHashForEach(snapshots-objs, virDomainSnapshotObjListCount, data); -return count; +return data.count; } static int virDomainSnapshotObjListSearchName(const void *payload, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 5f752ec..503fb58 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1335,8 +1335,10 @@ virDomainSnapshotObjPtr virDomainSnapshotAssignDef(virDomainSnapshotObjListPtr s int virDomainSnapshotObjListInit(virDomainSnapshotObjListPtr objs); int virDomainSnapshotObjListGetNames(virDomainSnapshotObjListPtr snapshots, - char **const names, int maxnames); -int virDomainSnapshotObjListNum(virDomainSnapshotObjListPtr snapshots); + char **const names, int maxnames, + unsigned int flags); +int virDomainSnapshotObjListNum(virDomainSnapshotObjListPtr snapshots, +unsigned int flags); virDomainSnapshotObjPtr virDomainSnapshotFindByName(const virDomainSnapshotObjListPtr snapshots, const char *name); void virDomainSnapshotObjListRemove(virDomainSnapshotObjListPtr snapshots, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3e6d653..e435a2f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8536,7 +8536,8 @@ static int qemuDomainSnapshotListNames(virDomainPtr domain, char **names, virDomainObjPtr vm = NULL; int n = -1; -virCheckFlags(VIR_DOMAIN_SNAPSHOT_LIST_METADATA, -1); +virCheckFlags(VIR_DOMAIN_SNAPSHOT_LIST_ROOTS | + VIR_DOMAIN_SNAPSHOT_LIST_METADATA, -1); qemuDriverLock(driver); vm = virDomainFindByUUID(driver-domains, domain-uuid); @@ -8548,7 +8549,8 @@ static int qemuDomainSnapshotListNames(virDomainPtr domain, char **names, goto cleanup; } -n = virDomainSnapshotObjListGetNames(vm-snapshots, names,
[libvirt] [PATCHv2 19/26] snapshot: prevent migration from stranding snapshot data
Migration is another case of stranding metadata. And since snapshot metadata is arbitrarily large, there's no way to shoehorn it into the migration cookie of migration v3. A future patch will make it possible to manually recreate the snapshot metadata on the destination. But even that is limited, since if we delete the snapshot metadata prior to migration, then we won't know the name of the current snapshot to pass along; and if we delete the snapshot metadata after migration and use the v3 migration cookie to pass along the name of the current snapshot, then we need a way to bypass the fact that this patch refuses migration with snapshot metadata present. So eventually, we may have to introduce migration protocol v4 that allows feature negotiation and an arbitrary number of handshake exchanges, so as to pass as many rpc calls as needed to transfer all the snapshot xml hierarchy. But all of that is thoughts for the future; for now, the best course of action is to quit early, rather than get into a funky state of stale metadata. * src/qemu/qemu_driver.c (qemudDomainMigratePerform) (qemuDomainMigrateBegin3, qemuDomainMigratePerform3): Add restriction. --- src/qemu/qemu_driver.c | 24 1 files changed, 24 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 027fdee..70fe607 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7723,6 +7723,7 @@ qemudDomainMigratePerform (virDomainPtr dom, virDomainObjPtr vm; int ret = -1; const char *dconnuri = NULL; +int nsnapshots; virCheckFlags(QEMU_MIGRATION_FLAGS, -1); @@ -7743,6 +7744,13 @@ qemudDomainMigratePerform (virDomainPtr dom, goto cleanup; } +if ((nsnapshots = virDomainSnapshotObjListNum(vm-snapshots, 0))) { +qemuReportError(VIR_ERR_OPERATION_INVALID, +_(cannot migrate domain with %d snapshots), +nsnapshots); +goto cleanup; +} + if (flags VIR_MIGRATE_PEER2PEER) { dconnuri = uri; uri = NULL; @@ -7819,6 +7827,7 @@ qemuDomainMigrateBegin3(virDomainPtr domain, struct qemud_driver *driver = domain-conn-privateData; virDomainObjPtr vm; char *xml = NULL; +int nsnapshots; virCheckFlags(QEMU_MIGRATION_FLAGS, NULL); @@ -7832,6 +7841,13 @@ qemuDomainMigrateBegin3(virDomainPtr domain, goto cleanup; } +if ((nsnapshots = virDomainSnapshotObjListNum(vm-snapshots, 0))) { +qemuReportError(VIR_ERR_OPERATION_INVALID, +_(cannot migrate domain with %d snapshots), +nsnapshots); +goto cleanup; +} + if ((flags VIR_MIGRATE_CHANGE_PROTECTION)) { if (qemuMigrationJobStart(driver, vm, QEMU_ASYNC_JOB_MIGRATION_OUT) 0) goto cleanup; @@ -7993,6 +8009,7 @@ qemuDomainMigratePerform3(virDomainPtr dom, struct qemud_driver *driver = dom-conn-privateData; virDomainObjPtr vm; int ret = -1; +int nsnapshots; virCheckFlags(QEMU_MIGRATION_FLAGS, -1); @@ -8006,6 +8023,13 @@ qemuDomainMigratePerform3(virDomainPtr dom, goto cleanup; } +if ((nsnapshots = virDomainSnapshotObjListNum(vm-snapshots, 0))) { +qemuReportError(VIR_ERR_OPERATION_INVALID, +_(cannot migrate domain with %d snapshots), +nsnapshots); +goto cleanup; +} + ret = qemuMigrationPerform(driver, dom-conn, vm, xmlin, dconnuri, uri, cookiein, cookieinlen, cookieout, cookieoutlen, -- 1.7.4.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 18/26] snapshot: teach virsh about new undefine flags
Similar to 'undefine --managed-save' (commit 83e849c1), we must assume that the old API is unsafe, and emulate it ourselves. Additionally, we have the wrinkle that while virDomainUndefineFlags and managed save cleanup were introduced in 0.9.4, it wasn't until 0.9.5 that snapshots block undefine of a domain. Simulate as much as possible with older servers, but the --snapshots-metadata support requires a server = 0.9.5. Same story for virDomainDestroyFlags, and virDomainShutdownFlags doesn't exist yet. Oh well. * tools/virsh.c (cmdUndefine, cmdDestroy, cmdShutdown): Add --snapshots-full and --snapshots-metadata flags. (vshRemoveAllSnapshots): New helper method. * tools/virsh.pod (undefine, destroy, shutdown): Document them. --- tools/virsh.c | 400 --- tools/virsh.pod | 36 +- 2 files changed, 386 insertions(+), 50 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index c094911..bb08d4c 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1410,6 +1410,59 @@ cmdDefine(vshControl *ctl, const vshCmd *cmd) return ret; } +/* Helper for undefine, shutdown, and destroy */ +static int +vshRemoveAllSnapshots(virDomainPtr dom, int nsnapshots, bool full) +{ +int ret = -1; +char **names; +int actual; +int i; +virDomainSnapshotPtr snapshot = NULL; +int flags = 0; + +/* It's likely that if virDomainUndefineFlags was unsupported to + * get us here in the first place, then + * VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY will also be + * unsupported. But better to hear that from the driver. + */ +if (!full) +flags |= VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY; + +if (VIR_ALLOC_N(names, nsnapshots) 0) +goto cleanup; + +actual = virDomainSnapshotListNames(dom, names, nsnapshots, 0); +if (actual 0) +goto cleanup; + +/* Sadly, this is an O(n) loop over a function call that is also a + * minimum of O(n), for a complexity of O(n^2), whereas newer + * servers that support the delete in the undefine action are + * O(n). Oh well. */ +for (i = 0; i actual; i++) { +if (snapshot) +virDomainSnapshotFree(snapshot); + +snapshot = virDomainSnapshotLookupByName(dom, names[i], 0); +if (snapshot == NULL) +continue; + +if (virDomainSnapshotDelete(snapshot, flags) 0) +goto cleanup; +} + +ret = 0; + +cleanup: +if (snapshot) +virDomainSnapshotFree(snapshot); +for (i = 0; i actual; i++) +VIR_FREE(names[i]); +VIR_FREE(names); +return ret; +} + /* * undefine command */ @@ -1422,6 +1475,10 @@ static const vshCmdInfo info_undefine[] = { static const vshCmdOptDef opts_undefine[] = { {domain, VSH_OT_DATA, VSH_OFLAG_REQ, N_(domain name or uuid)}, {managed-save, VSH_OT_BOOL, 0, N_(remove domain managed state file)}, +{snapshots-metadata, VSH_OT_BOOL, 0, + N_(remove all domain snapshot metadata, if inactive)}, +{snapshots-full, VSH_OT_BOOL, 0, + N_(remove all domain snapshot contents, if inactive)}, {NULL, 0, 0, NULL} }; @@ -1429,18 +1486,36 @@ static bool cmdUndefine(vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom; -bool ret = true; +bool ret = false; const char *name = NULL; +/* Flags to attempt. */ int flags = 0; -int managed_save = vshCommandOptBool(cmd, managed-save); +/* User-requested actions. */ +bool managed_save = vshCommandOptBool(cmd, managed-save); +bool snapshots_metadata = vshCommandOptBool(cmd, snapshots-metadata); +bool snapshots_full = vshCommandOptBool(cmd, snapshots-full); +/* Positive if these items exist. */ int has_managed_save = 0; +int has_snapshots_metadata = 0; +int has_snapshots = 0; +/* True if undefine will not strand data, even on older servers. */ +bool managed_save_safe = false; +bool snapshots_safe = false; int rc = -1; +int running; -if (managed_save) +if (managed_save) { flags |= VIR_DOMAIN_UNDEFINE_MANAGED_SAVE; - -if (!managed_save) -flags = -1; +managed_save_safe = true; +} +if (snapshots_metadata) { +flags |= VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA; +snapshots_safe = true; +} +if (snapshots_full) { +flags |= VIR_DOMAIN_UNDEFINE_SNAPSHOTS_FULL; +snapshots_safe = true; +} if (!vshConnectionUsability(ctl, ctl-conn)) return false; @@ -1448,61 +1523,124 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, name))) return false; -has_managed_save = virDomainHasManagedSaveImage(dom, 0); -if (has_managed_save 0) { -if (last_error-code != VIR_ERR_NO_SUPPORT) { -virshReportError(ctl); -virDomainFree(dom); -return false; -} else { +/* Do some flag manipulation. The goal here is to disable
[libvirt] [PATCHv2 04/26] snapshot: properly revert qemu to offline snapshots
qemuDomainSnapshotRevertInactive has the same FIXMEs as qemuDomainSnapshotCreateInactive, so algorithmic fixes to properly handle partial loop iterations should be applied later to both functions, but we're not making the situation any worse in this patch. * src/qemu/qemu_driver.c (qemuDomainRevertToSnapshot): Use qemu-img rather than 'qemu -loadvm' to revert to offline snapshot. (qemuDomainSnapshotRevertInactive): New helper. --- src/qemu/qemu_driver.c | 65 +-- 1 files changed, 56 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index aa53c63..989d21b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8734,6 +8734,52 @@ cleanup: return xml; } +/* The domain is expected to be locked and inactive. */ +static int +qemuDomainSnapshotRevertInactive(virDomainObjPtr vm, + virDomainSnapshotObjPtr snap) +{ +const char *qemuimgarg[] = { NULL, snapshot, -a, NULL, NULL, NULL }; +int ret = -1; +int i; + +qemuimgarg[0] = qemuFindQemuImgBinary(); +if (qemuimgarg[0] == NULL) { +/* qemuFindQemuImgBinary set the error */ +goto cleanup; +} + +qemuimgarg[3] = snap-def-name; + +for (i = 0; i vm-def-ndisks; i++) { +/* FIXME: we also need to handle LVM here */ +/* FIXME: if we fail halfway through this loop, we are in an + * inconsistent state. I'm not quite sure what to do about that + */ +if (vm-def-disks[i]-device == VIR_DOMAIN_DISK_DEVICE_DISK) { +if (!vm-def-disks[i]-driverType || +STRNEQ(vm-def-disks[i]-driverType, qcow2)) { +qemuReportError(VIR_ERR_OPERATION_INVALID, +_(Disk device '%s' does not support + snapshotting), +vm-def-disks[i]-info.alias); +goto cleanup; +} + +qemuimgarg[4] = vm-def-disks[i]-src; + +if (virRun(qemuimgarg, NULL) 0) +goto cleanup; +} +} + +ret = 0; + +cleanup: +VIR_FREE(qemuimgarg[0]); +return ret; +} + static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, unsigned int flags) { @@ -8840,14 +8886,9 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, VIR_DOMAIN_EVENT_STARTED_FROM_SNAPSHOT); } } else { -/* qemu is a little funny with running guests and the restoration - * of snapshots. If the snapshot was taken online, - * then after a loadvm monitor command, the VM is set running - * again. If the snapshot was taken offline, then after a loadvm - * monitor command the VM is left paused. Unpausing it leads to - * the memory state *before* the loadvm with the disk *after* the - * loadvm, which obviously is bound to corrupt something. - * Therefore we destroy the domain and set it to off in this case. +/* Newer qemu -loadvm refuses to revert to the state of a snapshot + * created by qemu-img snapshot -c. If the domain is running, we + * must take it offline; then do the revert using qemu-img. */ if (virDomainObjIsActive(vm)) { @@ -8857,6 +8898,12 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_FROM_SNAPSHOT); if (!vm-persistent) { +/* XXX it should be impossible to create an offline snapshot + * of a transient domain. Once we fix 'undefine' to convert + * a defined domain back to transient, that transition should + * be rejected if any offline snapshots exist. For now, we + * just stop the transient domain and quit, without reverting + * any disk state. */ if (qemuDomainObjEndJob(driver, vm) 0) virDomainRemoveInactive(driver-domains, vm); vm = NULL; @@ -8864,7 +8911,7 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, } } -if (qemuDomainSnapshotSetCurrentActive(vm, driver-snapshotDir) 0) +if (qemuDomainSnapshotRevertInactive(vm, snap) 0) goto endjob; } -- 1.7.4.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 15/26] snapshot: refactor some qemu code
Prepare for code sharing. No semantic change. * src/qemu/qemu_driver.c (qemuFindQemuImgBinary) (qemuDomainSnapshotWriteMetadata) (qemuDomainSnapshotDiscard): Float up. (qemuDomainSnapshotDiscardDescendant): Likewise, and rename... (qemuDomainSnapshotDiscardAll): ...for generic use. (qemuDomainSnapshotDelete): Update caller. --- src/qemu/qemu_driver.c | 370 1 files changed, 187 insertions(+), 183 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 754ab71..3977135 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1580,6 +1580,192 @@ cleanup: } +struct snap_remove { +struct qemud_driver *driver; +virDomainObjPtr vm; +bool metadata_only; +int err; +}; + +/* Locate an appropriate 'qemu-img' binary. */ +static char * +qemuFindQemuImgBinary(void) +{ +char *ret; + +ret = virFindFileInPath(kvm-img); +if (ret == NULL) +ret = virFindFileInPath(qemu-img); +if (ret == NULL) +qemuReportError(VIR_ERR_INTERNAL_ERROR, +%s, _(unable to find kvm-img or qemu-img)); + +return ret; +} + +static int qemuDomainSnapshotWriteMetadata(virDomainObjPtr vm, + virDomainSnapshotObjPtr snapshot, + char *snapshotDir) +{ +int fd = -1; +char *newxml = NULL; +int ret = -1; +char *snapDir = NULL; +char *snapFile = NULL; +char uuidstr[VIR_UUID_STRING_BUFLEN]; + +virUUIDFormat(vm-def-uuid, uuidstr); +newxml = virDomainSnapshotDefFormat(uuidstr, snapshot-def, 1); +if (newxml == NULL) { +virReportOOMError(); +return -1; +} + +if (virAsprintf(snapDir, %s/%s, snapshotDir, vm-def-name) 0) { +virReportOOMError(); +goto cleanup; +} +if (virFileMakePath(snapDir) 0) { +virReportSystemError(errno, _(cannot create snapshot directory '%s'), + snapDir); +goto cleanup; +} + +if (virAsprintf(snapFile, %s/%s.xml, snapDir, snapshot-def-name) 0) { +virReportOOMError(); +goto cleanup; +} +fd = open(snapFile, O_CREAT|O_TRUNC|O_WRONLY, S_IRUSR|S_IWUSR); +if (fd 0) { +qemuReportError(VIR_ERR_OPERATION_FAILED, +_(failed to create snapshot file '%s'), snapFile); +goto cleanup; +} +if (safewrite(fd, newxml, strlen(newxml)) != strlen(newxml)) { +virReportSystemError(errno, _(Failed to write snapshot data to %s), + snapFile); +goto cleanup; +} + +ret = 0; + +cleanup: +VIR_FREE(snapFile); +VIR_FREE(snapDir); +VIR_FREE(newxml); +VIR_FORCE_CLOSE(fd); +return ret; +} + +/* Discard one snapshot (or its metadata), without reparenting any children. */ +static int +qemuDomainSnapshotDiscard(struct qemud_driver *driver, + virDomainObjPtr vm, + virDomainSnapshotObjPtr snap, + bool metadata_only) +{ +const char *qemuimgarg[] = { NULL, snapshot, -d, NULL, NULL, NULL }; +char *snapFile = NULL; +int ret = -1; +int i; +qemuDomainObjPrivatePtr priv; +virDomainSnapshotObjPtr parentsnap; + +if (!metadata_only) { +if (!virDomainObjIsActive(vm)) { +qemuimgarg[0] = qemuFindQemuImgBinary(); +if (qemuimgarg[0] == NULL) +/* qemuFindQemuImgBinary set the error */ +goto cleanup; + +qemuimgarg[3] = snap-def-name; + +for (i = 0; i vm-def-ndisks; i++) { +/* FIXME: we also need to handle LVM here */ +if (vm-def-disks[i]-device == VIR_DOMAIN_DISK_DEVICE_DISK) { +if (!vm-def-disks[i]-driverType || +STRNEQ(vm-def-disks[i]-driverType, qcow2)) { +/* we continue on even in the face of error, since other + * disks in this VM may have this snapshot in place + */ +continue; +} + +qemuimgarg[4] = vm-def-disks[i]-src; + +if (virRun(qemuimgarg, NULL) 0) { +/* we continue on even in the face of error, since other + * disks in this VM may have this snapshot in place + */ +continue; +} +} +} +} else { +priv = vm-privateData; +qemuDomainObjEnterMonitorWithDriver(driver, vm); +/* we continue on even in the face of error */ +qemuMonitorDeleteSnapshot(priv-mon, snap-def-name); +qemuDomainObjExitMonitorWithDriver(driver, vm); +} +} + +if (virAsprintf(snapFile, %s/%s/%s.xml, driver-snapshotDir, +vm-def-name,
[libvirt] [PATCHv2 21/26] snapshot: allow full domain xml in snapshot
Just like VM saved state images (virsh save), snapshots MUST track the inactive domain xml to detect any ABI incompatibilities. The indentation is not perfect, but functionality comes before form. Later patches will actually supply a full domain; for now, this wires up the storage to support one, but doesn't ever generate one in dumpxml output. Happily, libvirt.c was already rejecting use of VIR_DOMAIN_XML_SECURE from read-only connections, even though before this patch, there was no one ever using that flag and there was no information to be secured by the use of that flag. * src/libvirt.c (virDomainSnapshotGetXMLDesc): Document flag. * src/conf/domain_conf.h (_virDomainSnapshotDef): Add member. (virDomainSnapshotDefParseString, virDomainSnapshotDefFormat): Update signature. * src/conf/domain_conf.c (virDomainSnapshotDefFree): Clean up. (virDomainSnapshotDefParseString): Optionally parse domain. (virDomainSnapshotDefFormat): Output full domain. * src/esx/esx_driver.c (esxDomainSnapshotCreateXML) (esxDomainSnapshotGetXMLDesc): Update callers. * src/vbox/vbox_tmpl.c (vboxDomainSnapshotCreateXML) (vboxDomainSnapshotGetXMLDesc): Likewise. * src/qemu/qemu_driver.c (qemuDomainSnapshotCreateXML) (qemuDomainSnapshotLoad, qemuDomainSnapshotGetXMLDesc) (qemuDomainSnapshotWriteMetadata): Likewise. * docs/formatsnapshot.html.in: Rework doc example. Based on a patch by Philipp Hahn. --- docs/formatsnapshot.html.in | 45 ++ src/conf/domain_conf.c | 50 +-- src/conf/domain_conf.h |7 +- src/esx/esx_driver.c|4 +- src/libvirt.c |7 +- src/qemu/qemu_driver.c | 27 --- src/vbox/vbox_tmpl.c|4 +- 7 files changed, 109 insertions(+), 35 deletions(-) diff --git a/docs/formatsnapshot.html.in b/docs/formatsnapshot.html.in index 79ed1d2..85dcc7f 100644 --- a/docs/formatsnapshot.html.in +++ b/docs/formatsnapshot.html.in @@ -46,27 +46,44 @@ snapshots. Readonly. /dd dtcodedomain/code/dt - ddThe domain that this snapshot was taken against. This - element contains exactly one child element, uuid. This - specifies the uuid of the domain that this snapshot was taken - against. Readonly. + ddThe domain that this snapshot was taken against. Older + versions of libvirt stored only a single child element, uuid; + reverting to a snapshot like this is risky if the current state + of the domain differs from the state that the domain was created + in, and requires the use of the VIR_DOMAIN_SNAPSHOT_REVERT_FORCE + flag. Newer versions of libvirt store the entire + inactive a href=formatdomain.htmldomain configuration/a at + the time of the snapshot. Readonly. /dd /dl -h2a name=exampleExample/a/h2 +h2a name=exampleExamples/a/h2 +pUsing this XML on creation:/p pre lt;domainsnapshotgt; - lt;namegt;os-updateslt;/namegt; lt;descriptiongt;Snapshot of OS install and updateslt;/descriptiongt; - lt;stategt;runninglt;/stategt; - lt;creationTimegt;1270477159lt;/creationTimegt; - lt;parentgt; -lt;namegt;bare-os-installlt;/namegt; - lt;/parentgt; - lt;domaingt; -lt;uuidgt;93a5c045-6457-2c09-e56c-927cdf34e178lt;/uuidgt; - lt;/domaingt; lt;/domainsnapshotgt;/pre + +pWill result in XML similar to this from +virDomainSnapshotGetXMLDesc:/p +pre +lt;domainsnapshotgt; + lt;namegt;1270477159lt;/namegt; + lt;descriptiongt;Snapshot of OS install and updateslt;/descriptiongt; + lt;stategt;runninglt;/stategt; + lt;creationTimegt;1270477159lt;/creationTimegt; + lt;parentgt; +lt;namegt;bare-os-installlt;/namegt; + lt;/parentgt; + lt;domaingt; +lt;namegt;fedoralt;/namegt; +lt;uuidgt;93a5c045-6457-2c09-e56c-927cdf34e178lt;/uuidgt; +lt;memorygt;1048576lt;/memorygt; +... +lt;/devicesgt; + lt;/domaingt; +lt;/domainsnapshotgt;/pre + /body /html diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b586bc8..ade8a02 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10960,11 +10960,17 @@ void virDomainSnapshotDefFree(virDomainSnapshotDefPtr def) VIR_FREE(def-name); VIR_FREE(def-description); VIR_FREE(def-parent); +virDomainDefFree(def-dom); VIR_FREE(def); } -virDomainSnapshotDefPtr virDomainSnapshotDefParseString(const char *xmlStr, -int newSnapshot) +/* If newSnapshot is true, caps, expectedVirtTypes, and flags are ignored. */ +virDomainSnapshotDefPtr +virDomainSnapshotDefParseString(const char *xmlStr, +int newSnapshot, +virCapsPtr caps, +unsigned int expectedVirtTypes, +unsigned int flags) {
[libvirt] [PATCHv2 23/26] snapshot: store qemu domain details in xml
When reverting to a snapshot, the inactive domain configuration has to be rolled back to what it was at the time of the snapshot. Additionally, if the VM is active and the snapshot was active, this now adds a failure if the two configurations are ABI incompatible, rather than risking qemu confusion. A future patch will add a VIR_DOMAIN_SNAPSHOT_FORCE flag, which will be required for three risky code paths - reverting to an older snapshot that lacked full domain information, reverting from running to a live snapshot that requires starting a new qemu process, and any reverting that stops a running vm. * src/qemu/qemu_driver.c (qemuDomainSnapshotCreateXML): Copy out domain. (qemuDomainRevertToSnapshot): Perform ABI compatibility checks. --- src/qemu/qemu_driver.c | 51 1 files changed, 51 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 13224ab..a8ea73d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8656,6 +8656,7 @@ static virDomainSnapshotPtr qemuDomainSnapshotCreateXML(virDomainPtr domain, { struct qemud_driver *driver = domain-conn-privateData; virDomainObjPtr vm = NULL; +char *xml = NULL; virDomainSnapshotObjPtr snap = NULL; virDomainSnapshotPtr snapshot = NULL; char uuidstr[VIR_UUID_STRING_BUFLEN]; @@ -8703,6 +8704,15 @@ static virDomainSnapshotPtr qemuDomainSnapshotCreateXML(virDomainPtr domain, vm-current_snapshot = NULL; } +/* Easiest way to clone inactive portion of vm-def is via + * conversion in and back out of xml. */ +if (!(xml = virDomainDefFormat(vm-def, (VIR_DOMAIN_XML_INACTIVE | + VIR_DOMAIN_XML_SECURE))) || +!(def-dom = virDomainDefParseString(driver-caps, xml, + QEMU_EXPECTED_VIRT_TYPES, + VIR_DOMAIN_XML_INACTIVE))) +goto cleanup; + /* actually do the snapshot */ if (!virDomainObjIsActive(vm)) { if (qemuDomainSnapshotCreateInactive(driver, vm, snap) 0) @@ -8733,6 +8743,7 @@ cleanup: virDomainSnapshotDefFree(def); virDomainObjUnlock(vm); } +VIR_FREE(xml); qemuDriverUnlock(driver); return snapshot; } @@ -8994,6 +9005,7 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, virDomainEventPtr event = NULL; qemuDomainObjPrivatePtr priv; int rc; +virDomainDefPtr config = NULL; virCheckFlags(0, -1); @@ -9024,7 +9036,30 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, * in the failure cases where we know there was no change? */ } +/* Prepare to copy the snapshot inactive xml as the config of this + * domain. Easiest way is by a round trip through xml. + * + * XXX Should domain snapshots track live xml rather + * than inactive xml? */ snap-def-current = true; +if (snap-def-dom) { +char *xml; +if (!(xml = virDomainDefFormat(snap-def-dom, + (VIR_DOMAIN_XML_INACTIVE | +VIR_DOMAIN_XML_SECURE +goto cleanup; +config = virDomainDefParseString(driver-caps, xml, + QEMU_EXPECTED_VIRT_TYPES, + VIR_DOMAIN_XML_INACTIVE); +VIR_FREE(xml); +if (!config) +goto cleanup; +} else { +/* XXX Fail if VIR_DOMAIN_REVERT_FORCE is not set, rather than + * blindly hoping for the best. */ +VIR_WARN(snapshot is lacking rollback information for domain '%s', + snap-def-name); +} if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) 0) goto cleanup; @@ -9037,6 +9072,15 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, * that is paused or running. We always pause before loadvm, * to have finer control. */ if (virDomainObjIsActive(vm)) { + +/* Check for ABI compatibility. */ +if (config !virDomainDefCheckABIStability(vm-def, config)) { +/* XXX Add VIR_DOMAIN_REVERT_FORCE to permit killing + * and restarting a new qemu, since loadvm monitor + * command won't work. */ +goto endjob; +} + priv = vm-privateData; if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) { if (qemuProcessStopCPUs(driver, vm, @@ -9064,7 +9108,12 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, * failed loadvm attempt? */ goto endjob; } +if (config) +virDomainObjAssignDef(vm, config, false); } else { +if (config) +
[libvirt] [PATCHv2 25/26] snapshot: reject transient disks where code is not ready
The previous patch introduced new config, but if a hypervisor does not support that new config, someone can write XML that does not behave as documented. This prevents some of those cases by explicitly rejecting transient disks for several hypervisors. Disk snapshots will require a new flag to actually affect a snapshot creation, so there's not much to reject there. * src/qemu/qemu_command.c (qemuBuildDriveStr): Reject transient disks for now. * src/libxl/libxl_conf.c (libxlMakeDisk): Likewise. * src/xenxs/xen_sxpr.c (xenFormatSxprDisk): Likewise. * src/xenxs/xen_xm.c (xenFormatXMDisk): Likewise. --- src/libxl/libxl_conf.c |5 + src/qemu/qemu_command.c |5 + src/xenxs/xen_sxpr.c|5 + src/xenxs/xen_xm.c |5 + 4 files changed, 20 insertions(+), 0 deletions(-) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 09f3be8..b9bce14 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -537,6 +537,11 @@ libxlMakeDisk(virDomainDefPtr def, virDomainDiskDefPtr l_disk, x_disk-unpluggable = 1; x_disk-readwrite = !l_disk-readonly; x_disk-is_cdrom = l_disk-device == VIR_DOMAIN_DISK_DEVICE_CDROM ? 1 : 0; +if (l_disk-transient) { +libxlError(VIR_ERR_INTERNAL_ERROR, %s, + _(libxenlight does not support transient disks)); +return -1; +} x_disk-domid = def-id; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 81e0525..35975f3 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1498,6 +1498,11 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk, if (disk-readonly qemuCapsGet(qemuCaps, QEMU_CAPS_DRIVE_READONLY)) virBufferAddLit(opt, ,readonly=on); +if (disk-transient) { +qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, +_(transient disks not supported yet)); +goto error; +} if (disk-driverType *disk-driverType != '\0' disk-type != VIR_DOMAIN_DISK_TYPE_DIR qemuCapsGet(qemuCaps, QEMU_CAPS_DRIVE_FORMAT)) diff --git a/src/xenxs/xen_sxpr.c b/src/xenxs/xen_sxpr.c index 1f5be5f..5f87ce9 100644 --- a/src/xenxs/xen_sxpr.c +++ b/src/xenxs/xen_sxpr.c @@ -1710,6 +1710,11 @@ xenFormatSxprDisk(virDomainDiskDefPtr def, virBufferAddLit(buf, (mode 'w!')); else virBufferAddLit(buf, (mode 'w')); +if (def-transient) { +XENXS_ERROR(VIR_ERR_CONFIG_UNSUPPORTED, +_(transient disks not supported yet)); +return -1; +} if (!isAttach) virBufferAddLit(buf, )); diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c index cb31226..03857c8 100644 --- a/src/xenxs/xen_xm.c +++ b/src/xenxs/xen_xm.c @@ -1164,6 +1164,11 @@ static int xenFormatXMDisk(virConfValuePtr list, virBufferAddLit(buf, ,!); else virBufferAddLit(buf, ,w); +if (disk-transient) { +XENXS_ERROR(VIR_ERR_CONFIG_UNSUPPORTED, +_(transient disks not supported yet)); +return -1; +} if (virBufferError(buf)) { virReportOOMError(); -- 1.7.4.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 17/26] snapshot: support new undefine flags in qemu
A nice benefit of deleting all snapshots at undefine time is that you don't have to do any reparenting or subtree identification - since everything goes, this is an O(n) process whereas using multiple virDomainSnapshotDelete calls would be O(n^2) or worse. * src/qemu/qemu_driver.c (qemuDomainDestroyFlags) (qemuDomainUndefineFlags): Honor new flags. --- src/qemu/qemu_driver.c | 57 +--- 1 files changed, 44 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f4a4786..027fdee 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1488,7 +1488,7 @@ static int qemuDomainShutdown(virDomainPtr dom) { } if (!vm-persistent -(nsnapshots = virDomainSnapshotObjListNum(vm-snapshots))) { +(nsnapshots = virDomainSnapshotObjListNum(vm-snapshots, 0))) { qemuReportError(VIR_ERR_OPERATION_INVALID, _(cannot delete transient domain with %d snapshots), nsnapshots); @@ -1777,7 +1777,8 @@ qemuDomainDestroyFlags(virDomainPtr dom, qemuDomainObjPrivatePtr priv; int nsnapshots; -virCheckFlags(0, -1); +virCheckFlags(VIR_DOMAIN_DESTROY_SNAPSHOTS_METADATA | + VIR_DOMAIN_DESTROY_SNAPSHOTS_FULL, -1); qemuDriverLock(driver); vm = virDomainFindByUUID(driver-domains, dom-uuid); @@ -1790,11 +1791,25 @@ qemuDomainDestroyFlags(virDomainPtr dom, } if (!vm-persistent -(nsnapshots = virDomainSnapshotObjListNum(vm-snapshots))) { -qemuReportError(VIR_ERR_OPERATION_INVALID, -_(cannot delete transient domain with %d snapshots), -nsnapshots); -goto cleanup; +(nsnapshots = virDomainSnapshotObjListNum(vm-snapshots, 0))) { +struct snap_remove rem; + +if ((flags (VIR_DOMAIN_DESTROY_SNAPSHOTS_METADATA | + VIR_DOMAIN_DESTROY_SNAPSHOTS_FULL)) == 0) { +qemuReportError(VIR_ERR_OPERATION_INVALID, +_(cannot delete transient domain with %d + snapshots), +nsnapshots); +goto cleanup; +} + +rem.driver = driver; +rem.vm = vm; +rem.metadata_only = !(flags VIR_DOMAIN_DESTROY_SNAPSHOTS_FULL); +rem.err = 0; +virHashForEach(vm-snapshots.objs, qemuDomainSnapshotDiscardAll, rem); +if (rem.err 0) +goto cleanup; } priv = vm-privateData; @@ -4918,7 +4933,9 @@ qemuDomainUndefineFlags(virDomainPtr dom, int ret = -1; int nsnapshots; -virCheckFlags(VIR_DOMAIN_UNDEFINE_MANAGED_SAVE, -1); +virCheckFlags(VIR_DOMAIN_UNDEFINE_MANAGED_SAVE | + VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA | + VIR_DOMAIN_UNDEFINE_SNAPSHOTS_FULL, -1); qemuDriverLock(driver); vm = virDomainFindByUUID(driver-domains, dom-uuid); @@ -4937,11 +4954,25 @@ qemuDomainUndefineFlags(virDomainPtr dom, qemuReportError(VIR_ERR_OPERATION_INVALID, %s, _(cannot delete active domain)); goto cleanup; -} else if ((nsnapshots = virDomainSnapshotObjListNum(vm-snapshots))) { -qemuReportError(VIR_ERR_OPERATION_INVALID, -_(cannot delete inactive domain with %d snapshots), -nsnapshots); -goto cleanup; +} else if ((nsnapshots = virDomainSnapshotObjListNum(vm-snapshots, 0))) { +struct snap_remove rem; + +if ((flags (VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA | + VIR_DOMAIN_UNDEFINE_SNAPSHOTS_FULL)) == 0) { +qemuReportError(VIR_ERR_OPERATION_INVALID, +_(cannot delete inactive domain with %d + snapshots), +nsnapshots); +goto cleanup; +} + +rem.driver = driver; +rem.vm = vm; +rem.metadata_only = !(flags VIR_DOMAIN_UNDEFINE_SNAPSHOTS_FULL); +rem.err = 0; +virHashForEach(vm-snapshots.objs, qemuDomainSnapshotDiscardAll, rem); +if (rem.err 0) +goto cleanup; } if (!vm-persistent) { -- 1.7.4.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 26/26] snapshot: wire up new qemu monitor command
No one uses this yet, but it will be important once virDomainSnapshotCreateXML learns a VIR_DOMAIN_SNAPSHOT_DISK_ONLY flag, and the xml allows passing in the new file names. * src/qemu/qemu_monitor.h (qemuMonitorDiskSnapshot): New prototype. * src/qemu/qemu_monitor_text.h (qemuMonitorTextDiskSnapshot): Likewise. * src/qemu/qemu_monitor_json.h (qemuMonitorJSONDiskSnapshot): Likewise. * src/qemu/qemu_monitor.c (qemuMonitorDiskSnapshot): New function. * src/qemu/qemu_monitor_json.c (qemuMonitorJSONDiskSnapshot): Likewise. --- src/qemu/qemu_monitor.c | 24 src/qemu/qemu_monitor.h |4 src/qemu/qemu_monitor_json.c | 33 + src/qemu/qemu_monitor_json.h |4 src/qemu/qemu_monitor_text.c | 40 src/qemu/qemu_monitor_text.h |4 6 files changed, 109 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index db6107c..efc49c4 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2391,6 +2391,30 @@ int qemuMonitorDeleteSnapshot(qemuMonitorPtr mon, const char *name) return ret; } +/* Use the snapshot_blkdev command to convert the existing file for + * device into a read-only backing file of a new qcow2 image located + * at file. */ +int +qemuMonitorDiskSnapshot(qemuMonitorPtr mon, const char *device, +const char *file) +{ +int ret; + +VIR_DEBUG(mon=%p, device=%s, file=%s, mon, device, file); + +if (!mon) { +qemuReportError(VIR_ERR_INVALID_ARG, %s, +_(monitor must not be NULL)); +return -1; +} + +if (mon-json) +ret = qemuMonitorJSONDiskSnapshot(mon, device, file); +else +ret = qemuMonitorTextDiskSnapshot(mon, device, file); +return ret; +} + int qemuMonitorArbitraryCommand(qemuMonitorPtr mon, const char *cmd, char **reply, diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index f241c9e..b988a72 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -447,6 +447,10 @@ int qemuMonitorCreateSnapshot(qemuMonitorPtr mon, const char *name); int qemuMonitorLoadSnapshot(qemuMonitorPtr mon, const char *name); int qemuMonitorDeleteSnapshot(qemuMonitorPtr mon, const char *name); +int qemuMonitorDiskSnapshot(qemuMonitorPtr mon, +const char *device, +const char *file); + int qemuMonitorArbitraryCommand(qemuMonitorPtr mon, const char *cmd, char **reply, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 2a9a078..d1fc188 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2671,6 +2671,39 @@ cleanup: return ret; } +int +qemuMonitorJSONDiskSnapshot(qemuMonitorPtr mon, const char *device, +const char *file) +{ +int ret; +virJSONValuePtr cmd; +virJSONValuePtr reply = NULL; + +cmd = qemuMonitorJSONMakeCommand(snapshot-blkdev-sync, + s:device, device, + s:snapshot-file, file, + NULL); +if (!cmd) +return -1; + +if ((ret = qemuMonitorJSONCommand(mon, cmd, reply)) 0) +goto cleanup; + +if (qemuMonitorJSONHasError(reply, CommandNotFound) +qemuMonitorCheckHMP(mon, snapshot_blkdev)) { +VIR_DEBUG(snapshot-blkdev-sync command not found, trying HMP); +ret = qemuMonitorTextDiskSnapshot(mon, device, file); +goto cleanup; +} + +ret = qemuMonitorJSONCheckError(cmd, reply); + +cleanup: +virJSONValueFree(cmd); +virJSONValueFree(reply); +return ret; +} + int qemuMonitorJSONArbitraryCommand(qemuMonitorPtr mon, const char *cmd_str, char **reply_str, diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 9512793..a538e9f 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -210,6 +210,10 @@ int qemuMonitorJSONCreateSnapshot(qemuMonitorPtr mon, const char *name); int qemuMonitorJSONLoadSnapshot(qemuMonitorPtr mon, const char *name); int qemuMonitorJSONDeleteSnapshot(qemuMonitorPtr mon, const char *name); +int qemuMonitorJSONDiskSnapshot(qemuMonitorPtr mon, +const char *device, +const char *file); + int qemuMonitorJSONArbitraryCommand(qemuMonitorPtr mon, const char *cmd_str, char **reply_str, diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 7bf733d..58e6470 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -2706,6 +2706,46
[libvirt] [PATCHv2 22/26] snapshot: update rng to support full domain in xml
* docs/schemas/domain.rng: Move guts... * docs/schemas/domaincommon.rng: ...to new file. * docs/schemas/domainsnapshot.rng: Allow new xml. * docs/schemas/Makefile.am (schema_DATA): Distribute new file. * tests/domainsnapshotxml2xmlout/full_domain.xml: New test. --- Email shortened by eliding some of the deletion lines (git move detected a copy rather than a rename, when rewriting domain.rng to wrap the moved file). docs/schemas/Makefile.am |1 + docs/schemas/domain.rng| 2555 +--- docs/schemas/{domain.rng = domaincommon.rng} |8 +- docs/schemas/domainsnapshot.rng| 14 +- tests/domainsnapshotxml2xmlout/full_domain.xml | 35 + 5 files changed, 49 insertions(+), 2564 deletions(-) copy docs/schemas/{domain.rng = domaincommon.rng} (99%) create mode 100644 tests/domainsnapshotxml2xmlout/full_domain.xml diff --git a/docs/schemas/Makefile.am b/docs/schemas/Makefile.am index 596c207..4413d9e 100644 --- a/docs/schemas/Makefile.am +++ b/docs/schemas/Makefile.am @@ -6,6 +6,7 @@ schema_DATA = \ basictypes.rng \ capability.rng \ domain.rng \ + domaincommon.rng \ domainsnapshot.rng \ interface.rng \ network.rng \ diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index dd8c41a..cf0be68 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -5,2558 +5,5 @@ ref name=domain/ /start - include href='basictypes.rng'/ - include href='storageencryption.rng'/ - include href='networkcommon.rng'/ - - !-- -description element, maybe placed anywhere under the root ... -data type=string - param name=pattern[a-zA-Z0-9_\.:]+/param -/data - /define + include href='domaincommon.rng'/ /grammar diff --git a/docs/schemas/domain.rng b/docs/schemas/domaincommon.rng similarity index 99% copy from docs/schemas/domain.rng copy to docs/schemas/domaincommon.rng index dd8c41a..756e892 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domaincommon.rng @@ -1,16 +1,12 @@ ?xml version=1.0? grammar xmlns=http://relaxng.org/ns/structure/1.0; datatypeLibrary=http://www.w3.org/2001/XMLSchema-datatypes; - !-- We handle only document defining a domain -- - start -ref name=domain/ - /start - + !-- domain-related definitions used in multiple grammars -- include href='basictypes.rng'/ include href='storageencryption.rng'/ include href='networkcommon.rng'/ !-- -description element, maybe placed anywhere under the root +description element, may be placed anywhere under the root -- define name=description element name=description diff --git a/docs/schemas/domainsnapshot.rng b/docs/schemas/domainsnapshot.rng index 410833f..a16d731 100644 --- a/docs/schemas/domainsnapshot.rng +++ b/docs/schemas/domainsnapshot.rng @@ -1,9 +1,12 @@ +?xml version=1.0? !-- A Relax NG schema for the libvirt domain snapshot properties XML format -- grammar xmlns=http://relaxng.org/ns/structure/1.0; start ref name='domainsnapshot'/ /start + include href='domaincommon.rng'/ + define name='domainsnapshot' element name='domainsnapshot' interleave @@ -36,11 +39,14 @@ /element /optional optional - element name='domain' -element name='uuid' - text/ + choice +element name='domain' + element name='uuid' +ref name=UUID/ + /element /element - /element +ref name='domain'/ + /choice /optional optional element name='parent' diff --git a/tests/domainsnapshotxml2xmlout/full_domain.xml b/tests/domainsnapshotxml2xmlout/full_domain.xml new file mode 100644 index 000..942bd7f --- /dev/null +++ b/tests/domainsnapshotxml2xmlout/full_domain.xml @@ -0,0 +1,35 @@ +domainsnapshot + namemy snap name/name + description!@#$%^/description + parent +nameearlier_snap/name + /parent + staterunning/state + creationTime1272917631/creationTime +domain type='qemu' + nameQEMUGuest1/name + uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid + memory219100/memory + currentMemory219100/currentMemory + vcpu cpuset='1-4,8-20,525'1/vcpu + os +type arch='i686' machine='pc'hvm/type +boot dev='hd'/ + /os + clock offset='utc'/ + on_poweroffdestroy/on_poweroff + on_rebootrestart/on_reboot + on_crashdestroy/on_crash + devices +emulator/usr/bin/qemu/emulator +disk type='block' device='disk' + source dev='/dev/HostVG/QEMUGuest1'/ + target dev='hda' bus='ide'/ + address type='drive' controller='0' bus='0' unit='0'/ +/disk +controller type='ide' index='0'/ +memballoon model='virtio'/ + /devices +/domain + active1/active +/domainsnapshot -- 1.7.4.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 16/26] snapshot: cache qemu-img location
As more clients start to want to know this information, doing a PATH stat walk and malloc for every client adds up. * src/qemu/qemu_conf.h (qemud_driver): Add member. * src/qemu/qemu_driver.c (qemudShutdown): Cleanup. (qemuFindQemuImgBinary): Add an argument, and cache result. (qemuDomainSnapshotDiscard, qemuDomainSnapshotCreateInactive) (qemuDomainSnapshotRevertInactive, qemuDomainSnapshotCreateXML) (qemuDomainRevertToSnapshot): Update callers. --- src/qemu/qemu_conf.h |1 + src/qemu/qemu_driver.c | 42 +- 2 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 0a60d32..5469a63 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -82,6 +82,7 @@ struct qemud_driver { char *cacheDir; char *saveDir; char *snapshotDir; +char *qemuImgBinary; unsigned int vncAutoUnixSocket : 1; unsigned int vncTLS : 1; unsigned int vncTLSx509verify : 1; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3977135..f4a4786 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -774,6 +774,7 @@ qemudShutdown(void) { VIR_FREE(qemu_driver-cacheDir); VIR_FREE(qemu_driver-saveDir); VIR_FREE(qemu_driver-snapshotDir); +VIR_FREE(qemu_driver-qemuImgBinary); VIR_FREE(qemu_driver-autoDumpPath); VIR_FREE(qemu_driver-vncTLSx509certdir); VIR_FREE(qemu_driver-vncListen); @@ -1588,19 +1589,19 @@ struct snap_remove { }; /* Locate an appropriate 'qemu-img' binary. */ -static char * -qemuFindQemuImgBinary(void) +static const char * +qemuFindQemuImgBinary(struct qemud_driver *driver) { -char *ret; - -ret = virFindFileInPath(kvm-img); -if (ret == NULL) -ret = virFindFileInPath(qemu-img); -if (ret == NULL) -qemuReportError(VIR_ERR_INTERNAL_ERROR, -%s, _(unable to find kvm-img or qemu-img)); +if (!driver-qemuImgBinary) { +driver-qemuImgBinary = virFindFileInPath(kvm-img); +if (!driver-qemuImgBinary) +driver-qemuImgBinary = virFindFileInPath(qemu-img); +if (!driver-qemuImgBinary) +qemuReportError(VIR_ERR_INTERNAL_ERROR, +%s, _(unable to find kvm-img or qemu-img)); +} -return ret; +return driver-qemuImgBinary; } static int qemuDomainSnapshotWriteMetadata(virDomainObjPtr vm, @@ -1673,7 +1674,7 @@ qemuDomainSnapshotDiscard(struct qemud_driver *driver, if (!metadata_only) { if (!virDomainObjIsActive(vm)) { -qemuimgarg[0] = qemuFindQemuImgBinary(); +qemuimgarg[0] = qemuFindQemuImgBinary(driver); if (qemuimgarg[0] == NULL) /* qemuFindQemuImgBinary set the error */ goto cleanup; @@ -1745,7 +1746,6 @@ qemuDomainSnapshotDiscard(struct qemud_driver *driver, cleanup: VIR_FREE(snapFile); -VIR_FREE(qemuimgarg[0]); return ret; } @@ -8488,14 +8488,15 @@ static int qemuDomainSnapshotIsAllowed(virDomainObjPtr vm) /* The domain is expected to be locked and inactive. */ static int -qemuDomainSnapshotCreateInactive(virDomainObjPtr vm, +qemuDomainSnapshotCreateInactive(struct qemud_driver *driver, + virDomainObjPtr vm, virDomainSnapshotObjPtr snap) { const char *qemuimgarg[] = { NULL, snapshot, -c, NULL, NULL, NULL }; int ret = -1; int i; -qemuimgarg[0] = qemuFindQemuImgBinary(); +qemuimgarg[0] = qemuFindQemuImgBinary(driver); if (qemuimgarg[0] == NULL) { /* qemuFindQemuImgBinary set the error */ goto cleanup; @@ -8528,7 +8529,6 @@ qemuDomainSnapshotCreateInactive(virDomainObjPtr vm, ret = 0; cleanup: -VIR_FREE(qemuimgarg[0]); return ret; } @@ -8639,7 +8639,7 @@ static virDomainSnapshotPtr qemuDomainSnapshotCreateXML(virDomainPtr domain, /* actually do the snapshot */ if (!virDomainObjIsActive(vm)) { -if (qemuDomainSnapshotCreateInactive(vm, snap) 0) +if (qemuDomainSnapshotCreateInactive(driver, vm, snap) 0) goto cleanup; } else { if (qemuDomainSnapshotCreateActive(domain-conn, driver, @@ -8873,14 +8873,15 @@ cleanup: /* The domain is expected to be locked and inactive. */ static int -qemuDomainSnapshotRevertInactive(virDomainObjPtr vm, +qemuDomainSnapshotRevertInactive(struct qemud_driver *driver, + virDomainObjPtr vm, virDomainSnapshotObjPtr snap) { const char *qemuimgarg[] = { NULL, snapshot, -a, NULL, NULL, NULL }; int ret = -1; int i; -qemuimgarg[0] = qemuFindQemuImgBinary(); +qemuimgarg[0] = qemuFindQemuImgBinary(driver); if (qemuimgarg[0] == NULL) { /* qemuFindQemuImgBinary set the error */ goto cleanup; @@ -8913,7 +8914,6 @@ qemuDomainSnapshotRevertInactive(virDomainObjPtr vm, ret = 0;
[libvirt] [PATCHv2 06/26] snapshot: only pass snapshot to qemu command line when reverting
Changing the current vm, and writing that change to the file system, all before a new qemu starts, is risky; it's hard to roll back if starting the new qemu fails for some reason. Instead of abusing vm-current_snapshot and making the command line generator decide whether the current snapshot warrants using -loadvm, it is better to just directly pass a snapshot all the way through the call chain if it is to be loaded. This frees up the last use of snapshot-def-active for qemu's use, so the next patch can repurpose that field for tracking which snapshot is current. * src/qemu/qemu_command.c (qemuBuildCommandLine): Don't use active field of snapshot. * src/qemu/qemu_process.c (qemuProcessStart): Add a parameter. * src/qemu/qemu_process.h (qemuProcessStart): Update prototype. * src/qemu/qemu_migration.c (qemuMigrationPrepareAny): Update callers. * src/qemu/qemu_driver.c (qemudDomainCreate) (qemuDomainSaveImageStartVM, qemuDomainObjStart) (qemuDomainRevertToSnapshot): Likewise. (qemuDomainSnapshotSetCurrentActive) (qemuDomainSnapshotSetCurrentInactive): Delete unused functions. --- src/qemu/qemu_command.c |7 ++--- src/qemu/qemu_driver.c| 47 src/qemu/qemu_migration.c |2 +- src/qemu/qemu_process.c | 10 +--- src/qemu/qemu_process.h |1 + 5 files changed, 12 insertions(+), 55 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index dbfc7d9..81e0525 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2866,7 +2866,7 @@ qemuBuildCommandLine(virConnectPtr conn, virBitmapPtr qemuCaps, const char *migrateFrom, int migrateFd, - virDomainSnapshotObjPtr current_snapshot, + virDomainSnapshotObjPtr snapshot, enum virVMOperationType vmop) { int i; @@ -4782,9 +4782,8 @@ qemuBuildCommandLine(virConnectPtr conn, } } -if (current_snapshot current_snapshot-def-active) -virCommandAddArgList(cmd, -loadvm, current_snapshot-def-name, - NULL); +if (snapshot) +virCommandAddArgList(cmd, -loadvm, snapshot-def-name, NULL); if (def-namespaceData) { qemuDomainCmdlineDefPtr qemucmd; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b83e1f0..7426690 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -804,12 +804,6 @@ qemudShutdown(void) { } -static int qemuDomainSnapshotSetCurrentActive(virDomainObjPtr vm, - char *snapshotDir); -static int qemuDomainSnapshotSetCurrentInactive(virDomainObjPtr vm, -char *snapshotDir); - - static virDrvOpenStatus qemudOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, unsigned int flags) @@ -1297,7 +1291,7 @@ static virDomainPtr qemudDomainCreate(virConnectPtr conn, const char *xml, if (qemuProcessStart(conn, driver, vm, NULL, (flags VIR_DOMAIN_START_PAUSED) != 0, (flags VIR_DOMAIN_START_AUTODESTROY) != 0, - -1, NULL, VIR_VM_OP_CREATE) 0) { + -1, NULL, NULL, VIR_VM_OP_CREATE) 0) { virDomainAuditStart(vm, booted, false); if (qemuDomainObjEndJob(driver, vm) 0) virDomainRemoveInactive(driver-domains, @@ -3920,7 +3914,7 @@ qemuDomainSaveImageStartVM(virConnectPtr conn, /* Set the migration source and start it up. */ ret = qemuProcessStart(conn, driver, vm, stdio, true, - false, *fd, path, VIR_VM_OP_RESTORE); + false, *fd, path, NULL, VIR_VM_OP_RESTORE); if (intermediatefd != -1) { if (ret 0) { @@ -4462,7 +4456,7 @@ qemuDomainObjStart(virConnectPtr conn, } ret = qemuProcessStart(conn, driver, vm, NULL, start_paused, - autodestroy, -1, NULL, VIR_VM_OP_CREATE); + autodestroy, -1, NULL, NULL, VIR_VM_OP_CREATE); virDomainAuditStart(vm, booted, ret = 0); if (ret = 0) { virDomainEventPtr event = @@ -8314,32 +8308,6 @@ cleanup: return ret; } -static int qemuDomainSnapshotSetCurrentActive(virDomainObjPtr vm, - char *snapshotDir) -{ -if (vm-current_snapshot) { -vm-current_snapshot-def-active = 1; - -return qemuDomainSnapshotWriteMetadata(vm, vm-current_snapshot, - snapshotDir); -} - -return 0; -} - -static int qemuDomainSnapshotSetCurrentInactive(virDomainObjPtr vm, -char *snapshotDir) -{ -if (vm-current_snapshot) { -vm-current_snapshot-def-active = 0; - -return
[libvirt] [PATCHv2 20/26] snapshot: refactor domain xml output
Minor semantic change - allow domain xml to be generated in place within a larger buffer, rather than having to go through a temporary string. * src/conf/domain_conf.c (virDomainDefFormatInternal): Add parameter. (virDomainDefFormat, virDomainObjFormat): Update callers. --- src/conf/domain_conf.c | 229 --- 1 files changed, 117 insertions(+), 112 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 4bf3541..b586bc8 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9908,12 +9908,15 @@ verify(((VIR_DOMAIN_XML_INTERNAL_STATUS | DUMPXML_FLAGS) == 0); /* This internal version can accept VIR_DOMAIN_XML_INTERNAL_*, - * whereas the public version cannot. */ -static char * + * whereas the public version cannot. Also, it appends to an existing + * buffer, rather than flattening to string. Return -1 on failure. */ +static int virDomainDefFormatInternal(virDomainDefPtr def, - unsigned int flags) + unsigned int flags, + virBufferPtr buf) { -virBuffer buf = VIR_BUFFER_INITIALIZER; +/* XXX Also need to take an indentation parameter - either int or + * string prefix, so that snapshot xml gets uniform indentation. */ unsigned char *uuid; char uuidstr[VIR_UUID_STRING_BUFLEN]; const char *type = NULL; @@ -9922,7 +9925,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, virCheckFlags(DUMPXML_FLAGS | VIR_DOMAIN_XML_INTERNAL_STATUS | VIR_DOMAIN_XML_INTERNAL_ACTUAL_NET, - NULL); + -1); if (!(type = virDomainVirtTypeToString(def-virtType))) { virDomainReportError(VIR_ERR_INTERNAL_ERROR, @@ -9933,99 +9936,99 @@ virDomainDefFormatInternal(virDomainDefPtr def, if (def-id == -1) flags |= VIR_DOMAIN_XML_INACTIVE; -virBufferAsprintf(buf, domain type='%s', type); +virBufferAsprintf(buf, domain type='%s', type); if (!(flags VIR_DOMAIN_XML_INACTIVE)) -virBufferAsprintf(buf, id='%d', def-id); +virBufferAsprintf(buf, id='%d', def-id); if (def-namespaceData def-ns.href) -virBufferAsprintf(buf, %s, (def-ns.href)()); -virBufferAddLit(buf, \n); +virBufferAsprintf(buf, %s, (def-ns.href)()); +virBufferAddLit(buf, \n); -virBufferEscapeString(buf, name%s/name\n, def-name); +virBufferEscapeString(buf, name%s/name\n, def-name); uuid = def-uuid; virUUIDFormat(uuid, uuidstr); -virBufferAsprintf(buf, uuid%s/uuid\n, uuidstr); +virBufferAsprintf(buf, uuid%s/uuid\n, uuidstr); if (def-description) -virBufferEscapeString(buf, description%s/description\n, +virBufferEscapeString(buf, description%s/description\n, def-description); -virBufferAsprintf(buf, memory%lu/memory\n, def-mem.max_balloon); -virBufferAsprintf(buf, currentMemory%lu/currentMemory\n, +virBufferAsprintf(buf, memory%lu/memory\n, def-mem.max_balloon); +virBufferAsprintf(buf, currentMemory%lu/currentMemory\n, def-mem.cur_balloon); /* add blkiotune only if there are any */ if (def-blkio.weight) { -virBufferAsprintf(buf, blkiotune\n); -virBufferAsprintf(buf, weight%u/weight\n, +virBufferAsprintf(buf, blkiotune\n); +virBufferAsprintf(buf, weight%u/weight\n, def-blkio.weight); -virBufferAsprintf(buf, /blkiotune\n); +virBufferAsprintf(buf, /blkiotune\n); } /* add memtune only if there are any */ if (def-mem.hard_limit || def-mem.soft_limit || def-mem.min_guarantee || def-mem.swap_hard_limit) -virBufferAsprintf(buf, memtune\n); +virBufferAsprintf(buf, memtune\n); if (def-mem.hard_limit) { -virBufferAsprintf(buf, hard_limit%lu/hard_limit\n, +virBufferAsprintf(buf, hard_limit%lu/hard_limit\n, def-mem.hard_limit); } if (def-mem.soft_limit) { -virBufferAsprintf(buf, soft_limit%lu/soft_limit\n, +virBufferAsprintf(buf, soft_limit%lu/soft_limit\n, def-mem.soft_limit); } if (def-mem.min_guarantee) { -virBufferAsprintf(buf, min_guarantee%lu/min_guarantee\n, +virBufferAsprintf(buf, min_guarantee%lu/min_guarantee\n, def-mem.min_guarantee); } if (def-mem.swap_hard_limit) { -virBufferAsprintf(buf, swap_hard_limit%lu/swap_hard_limit\n, +virBufferAsprintf(buf, swap_hard_limit%lu/swap_hard_limit\n, def-mem.swap_hard_limit); } if (def-mem.hard_limit || def-mem.soft_limit || def-mem.min_guarantee || def-mem.swap_hard_limit) -virBufferAsprintf(buf, /memtune\n); +virBufferAsprintf(buf,
[libvirt] [PATCHv2 07/26] snapshot: track current snapshot across restarts
Audit all changes to the qemu vm-current_snapshot, and make them update the saved xml file for both the previous and the new snapshot, so that there is always at most one snapshot with active1/active in the xml, and that snapshot is used as the current snapshot even across libvirtd restarts. * src/conf/domain_conf.h (_virDomainSnapshotDef): Alter member type and name. * src/conf/domain_conf.c (virDomainSnapshotDefParseString) (virDomainSnapshotDefFormat): Update clients. * docs/schemas/domainsnapshot.rng: Tighten rng. * src/qemu/qemu_driver.c (qemuDomainSnapshotLoad): Reload current snapshot. (qemuDomainSnapshotCreateXML, qemuDomainRevertToSnapshot) (qemuDomainSnapshotDiscard): Track current snapshot. --- docs/schemas/domainsnapshot.rng |5 ++- src/conf/domain_conf.c |6 ++- src/conf/domain_conf.h |4 +- src/qemu/qemu_driver.c | 79 +- 4 files changed, 71 insertions(+), 23 deletions(-) diff --git a/docs/schemas/domainsnapshot.rng b/docs/schemas/domainsnapshot.rng index 86bab0b..410833f 100644 --- a/docs/schemas/domainsnapshot.rng +++ b/docs/schemas/domainsnapshot.rng @@ -29,7 +29,10 @@ /optional optional element name='active' -text/ +choice + value0/value + value1/value +/choice /element /optional optional diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ce1f3c5..7793a13 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10967,6 +10967,7 @@ virDomainSnapshotDefPtr virDomainSnapshotDefParseString(const char *xmlStr, virDomainSnapshotDefPtr ret = NULL; char *creation = NULL, *state = NULL; struct timeval tv; +int active; xml = virXMLParse(NULL, xmlStr, domainsnapshot.xml); if (!xml) { @@ -11030,11 +11031,12 @@ virDomainSnapshotDefPtr virDomainSnapshotDefParseString(const char *xmlStr, goto cleanup; } -if (virXPathLong(string(./active), ctxt, def-active) 0) { +if (virXPathInt(string(./active), ctxt, active) 0) { virDomainReportError(VIR_ERR_INTERNAL_ERROR, %s, _(Could not find 'active' element)); goto cleanup; } +def-current = active != 0; } else def-creationTime = tv.tv_sec; @@ -11076,7 +11078,7 @@ char *virDomainSnapshotDefFormat(char *domain_uuid, virBufferAsprintf(buf, uuid%s/uuid\n, domain_uuid); virBufferAddLit(buf, /domain\n); if (internal) -virBufferAsprintf(buf, active%ld/active\n, def-active); +virBufferAsprintf(buf, active%d/active\n, def-current); virBufferAddLit(buf, /domainsnapshot\n); if (virBufferError(buf)) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 2cc9b06..8382d28 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1297,13 +1297,15 @@ enum virDomainTaintFlags { typedef struct _virDomainSnapshotDef virDomainSnapshotDef; typedef virDomainSnapshotDef *virDomainSnapshotDefPtr; struct _virDomainSnapshotDef { +/* Public XML. */ char *name; char *description; char *parent; long long creationTime; /* in seconds */ int state; -long active; +/* Internal use. */ +bool current; /* At most one snapshot in the list should have this set */ }; typedef struct _virDomainSnapshotObj virDomainSnapshotObj; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7426690..76c5549 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -294,6 +294,7 @@ static void qemuDomainSnapshotLoad(void *payload, char *fullpath; virDomainSnapshotDefPtr def = NULL; virDomainSnapshotObjPtr snap = NULL; +virDomainSnapshotObjPtr current = NULL; char ebuf[1024]; virDomainObjLock(vm); @@ -339,7 +340,8 @@ static void qemuDomainSnapshotLoad(void *payload, def = virDomainSnapshotDefParseString(xmlStr, 0); if (def == NULL) { /* Nothing we can do here, skip this one */ -VIR_ERROR(_(Failed to parse snapshot XML from file '%s'), fullpath); +VIR_ERROR(_(Failed to parse snapshot XML from file '%s'), + fullpath); VIR_FREE(fullpath); VIR_FREE(xmlStr); continue; @@ -348,12 +350,22 @@ static void qemuDomainSnapshotLoad(void *payload, snap = virDomainSnapshotAssignDef(vm-snapshots, def); if (snap == NULL) { virDomainSnapshotDefFree(def); +} else if (snap-def-current) { +current = snap; +if (!vm-current_snapshot) +vm-current_snapshot = snap; } VIR_FREE(fullpath); VIR_FREE(xmlStr); } +if (vm-current_snapshot != current) { +VIR_ERROR(_(Too many snapshots claiming to be current for domain %s), +
[libvirt] [PATCHv2 24/26] snapshot: add 2 attributes to domain xml for disks
As discussed here: https://www.redhat.com/archives/libvir-list/2011-August/msg00361.html https://www.redhat.com/archives/libvir-list/2011-August/msg00552.html Adds: devices disk type=... snapshot='no|internal|external' ... transient/ /disk /devices * docs/schemas/domaincommon.rng (snapshot): New define. (disk): Add snapshot and persistent attributes. * docs/formatdomain.html.in: Document them. * src/conf/domain_conf.h (virDomainDiskSnapshot): New enum. (_virDomainDiskDef): New fields. --- docs/formatdomain.html.in | 40 docs/schemas/domaincommon.rng | 17 + src/conf/domain_conf.c| 35 +-- src/conf/domain_conf.h| 12 src/libvirt_private.syms |2 ++ 5 files changed, 100 insertions(+), 6 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index f46771d..911dee5 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -889,7 +889,7 @@ pre ... lt;devicesgt; -lt;disk type='file'gt; +lt;disk type='file' snapshot='external'gt; lt;driver name=tap type=aio cache=default/gt; lt;source file='/var/lib/xen/images/fv0'/gt; lt;target dev='hda' bus='ide'/gt; @@ -910,8 +910,14 @@ lt;/sourcegt; lt;target dev=hdb bus=ide/gt; lt;boot order='1'/gt; + lt;transient/gt; lt;address type='drive' controller='0' bus='1' unit='0'/gt; lt;/diskgt; +lt;disk type='block' device='cdrom'gt; + lt;driver name='qemu' type='raw'/gt; + lt;target def='hdc' bus='ide'/gt; + lt;readonly/gt; +lt;/diskgt; lt;/devicesgt; .../pre @@ -923,9 +929,23 @@ and refers to the underlying source for the disk. The optional codedevice/code attribute indicates how the disk is to be exposed to the guest OS. Possible values for this attribute are floppy, disk -and cdrom, defaulting to disk. -span class=sinceSince 0.0.3; device attribute since 0.1.4; -network attribute since 0.8.7/span/dd +and cdrom, defaulting to disk. The +optional codesnapshot/code attribute indicates the default +behavior of the disk during disk snapshots: internal +requires a file format such as qcow2 that can store both the +snapshot and the data changes since the snapshot; +external will separate the snapshot from the live data; and +no means the disk will not participate in snapshots. +Read-only disks default to no, while the default for other +disks depends on the hypervisor's capabilities. Some +hypervisors allow a per-snapshot choice as well, +during a href=formatsnapshot.htmldomain snapshot +creation/a. Not all snapshot modes are supported; +for example, codesnapshot='yes'/code with a transient disk +generally does not make sense. span class=sinceSince 0.0.3; +device attribute since 0.1.4; +network attribute since 0.8.7; snapshot since +0.9.5/span/dd dtcodesource/code/dt ddIf the disk codetype/code is file, then the codefile/code attribute specifies the fully-qualified @@ -1032,11 +1052,23 @@ the a href=formatstorageencryption.htmlStorage Encryption/a page for more information. /dd + dtcodereadonly/code/dt + ddIf present, this indicates the device cannot be modified by +the guest. For now, this is the default for disks with +attribute codetype='cdrom'/code. + /dd dtcodeshareable/code/dt ddIf present, this indicates the device is expected to be shared between domains (assuming the hypervisor and OS support this), which means that caching should be deactivated for that device. /dd + dtcodetransient/code/dt + ddIf present, this indicates that changes to the device +contents should be reverted automatically when the guest +exits. With some hypervisors, marking a disk transient +prevents the domain from participating in migration or +snapshots. span class=sinceSince 0.9.5/span + /dd dtcodeserial/code/dt ddIf present, this specify serial number of virtual hard drive. For example, it may look diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 756e892..0af9e0f 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -617,6 +617,11 @@ /element /optional optional + element name=transient +empty/ + /element +/optional +optional element name=serial ref name=diskSerial/ /element @@ -628,6 +633,15 @@ ref name=address/ /optional /define + define name=snapshot +attribute name=snapshot + choice +valueno/value +valueinternal/value +valueexternal/value +
Re: [libvirt] RFC (V2) New virDomainBlockPull API family to libvirt
On Mon, Aug 15, 2011 at 8:36 PM, Adam Litke a...@us.ibm.com wrote: On 08/14/2011 11:40 PM, Zhi Yong Wu wrote: HI, Deniel and Adam. Have the patchset been merged into libvirt upstream? Yes they have. However, the functionality is still missing from qemu. The two communities have agreed upon the interface and semantics, but work continues on the qemu implementation. Let me know if you would like a link to some qemu patches that support this functionality for qed images. Sure, If you share it with me, at the same time learning your libvirt API, i can also sample this feature.:) anyway, thanks, Adam. -- Adam Litke IBM Linux Technology Center -- Regards, Zhi Yong Wu -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] RFC (V2) New virDomainBlockPull API family to libvirt
On Tue, Aug 16, 2011 at 6:52 AM, Stefan Hajnoczi stefa...@gmail.com wrote: On Mon, Aug 15, 2011 at 1:36 PM, Adam Litke a...@us.ibm.com wrote: On 08/14/2011 11:40 PM, Zhi Yong Wu wrote: HI, Deniel and Adam. Have the patchset been merged into libvirt upstream? Yes they have. However, the functionality is still missing from qemu. The two communities have agreed upon the interface and semantics, but work continues on the qemu implementation. Let me know if you would like a link to some qemu patches that support this functionality for qed images. I also have a series to put these commands into QEMU without any image format support. They just return NotSupported but it puts the commands into QEMU so we can run the libvirt commands against them. Without image format support, it will not be a nice sample for us.:) Why did you not implement them? Will send those patches to qemu-devel soon. Stefan -- Regards, Zhi Yong Wu -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] about Virsh Command Reference
On Tue, Jul 26, 2011 at 11:03:52AM +0800, Hu Tao wrote: Hi, We now have libvirt advanced at 0.9.3, but there is almost no progress has been made on Virsh Command Reference. I'm now starting on updating Virsh Command Reference to get it synced with libvirt/virsh, but since I'm not a native-English speaker, there must be grammers, typos, etc. in the documents written by me, so any comments on these are appreciated. Besides, comments on other errors, such as example usages, are also welcome. In this series, docs for two cmmands are updated. I'll post more patches for other commands subsequently. Hu Tao (2): update documentation for command attach-disk update documentation for command detach-disk source/attach-disk.xml | 292 +++- source/detach-disk.xml | 233 +- 2 files changed, 517 insertions(+), 8 deletions(-) -- 1.7.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list ping -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Get memory balloon info correctly for text monitor
于 2011年08月16日 00:27, Adam Litke 写道: On 08/15/2011 08:23 AM, Osier Yang wrote: 于 2011年08月15日 21:58, Osier Yang 写道: * src/qemu/qemu_monitor_text.c: BALLOON_PREFIX was defined as balloon: actual=, which cause actual= is stripped early before the real parsing. This patch changes BALLOON_PREFIX into balloon: , and modifies related functions, also renames qemuMonitorParseExtraBalloonInfo to qemuMonitorParseBalloonInfo, as after the changing, it parses all the info returned by info balloon. Forgot to mention the problem, e.g. virsh dommemstat $domain returns empty result. That is because qemu has disabled stats reporting and so the extra fields are not present in the info balloon response. Yes, I made a patch for this before (commit 41514f7b), but I didn't realize the actual= is stripped early before the parsing, so with the commit, one will get value of actual at least in QMP mode even the extra fields are disabled by qemu. But for text monitor, the problem still exists, this patch is to fix it. Osier -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Get memory balloon info correctly for text monitor
于 2011年08月16日 00:40, Adam Litke 写道: Hi Osier, Just to be clear, this is a cleanup not a bugfix right? The current code should be working properly as written. No, virDomainMemoryStats will return empty result if libvirt communicates to qemu monitor in text mode. Actually this is reported by a upstream Debian user, It seems to me packager of Debian compiles libvirt without QMP support. On 08/15/2011 08:58 AM, Osier Yang wrote: * src/qemu/qemu_monitor_text.c: BALLOON_PREFIX was defined as balloon: actual=, which cause actual= is stripped early before the real parsing. This patch changes BALLOON_PREFIX into balloon: , and modifies related functions, also renames qemuMonitorParseExtraBalloonInfo to qemuMonitorParseBalloonInfo, as after the changing, it parses all the info returned by info balloon. --- src/qemu/qemu_monitor_text.c | 47 + 1 files changed, 29 insertions(+), 18 deletions(-) diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 7bf733d..d17d863 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -547,8 +547,12 @@ static int parseMemoryStat(char **text, unsigned int tag, return 0; } -/* Convert bytes to kilobytes for libvirt */ switch (tag) { +/* Convert megabytes to kilobytes for libvirt */ +case VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON: +value = value 10; +break; +/* Convert bytes to kilobytes for libvirt */ case VIR_DOMAIN_MEMORY_STAT_SWAP_IN: case VIR_DOMAIN_MEMORY_STAT_SWAP_OUT: case VIR_DOMAIN_MEMORY_STAT_UNUSED: @@ -565,15 +569,17 @@ static int parseMemoryStat(char **text, unsigned int tag, /* The reply from the 'info balloon' command may contain additional memory * statistics in the form: '[,tag=val]*' */ -static int qemuMonitorParseExtraBalloonInfo(char *text, -virDomainMemoryStatPtr stats, -unsigned int nr_stats) +static int qemuMonitorParseBalloonInfo(char *text, + virDomainMemoryStatPtr stats, + unsigned int nr_stats) { char *p = text; unsigned int nr_stats_found = 0; while (*p nr_stats_found nr_stats) { -if (parseMemoryStat(p, VIR_DOMAIN_MEMORY_STAT_SWAP_IN, +if (parseMemoryStat(p, VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON, +actual=,stats[nr_stats_found]) || +parseMemoryStat(p, VIR_DOMAIN_MEMORY_STAT_SWAP_IN, ,mem_swapped_in=,stats[nr_stats_found]) || parseMemoryStat(p, VIR_DOMAIN_MEMORY_STAT_SWAP_OUT, ,mem_swapped_out=,stats[nr_stats_found]) || According to this code (and actual monitor behavior) 'actual' always appears and has to come first. Therefore, I would still parse the 'actual' stat outside of the while loop. @@ -584,9 +590,7 @@ static int qemuMonitorParseExtraBalloonInfo(char *text, parseMemoryStat(p, VIR_DOMAIN_MEMORY_STAT_UNUSED, ,free_mem=,stats[nr_stats_found]) || parseMemoryStat(p, VIR_DOMAIN_MEMORY_STAT_AVAILABLE, -,total_mem=,stats[nr_stats_found]) || -parseMemoryStat(p, VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON, -,actual=,stats[nr_stats_found])) +,total_mem=,stats[nr_stats_found])) nr_stats_found++; /* Skip to the next label. When *p is ',' the last match attempt @@ -602,7 +606,7 @@ static int qemuMonitorParseExtraBalloonInfo(char *text, /* The reply from QEMU contains 'ballon: actual=421' where value is in MB */ -#define BALLOON_PREFIX balloon: actual= +#define BALLOON_PREFIX balloon: /* * Returns: 0 if balloon not supported, +1 if balloon query worked @@ -625,13 +629,22 @@ int qemuMonitorTextGetBalloonInfo(qemuMonitorPtr mon, unsigned int memMB; char *end; offset += strlen(BALLOON_PREFIX); -if (virStrToLong_ui(offset,end, 10,memMB) 0) { -qemuReportError(VIR_ERR_OPERATION_FAILED, -_(could not parse memory balloon allocation from '%s'), reply); -goto cleanup; + +if (STRPREFIX(offset, actual=)) { +offset += strlen(actual=); + +if (virStrToLong_ui(offset,end, 10,memMB) 0) { +qemuReportError(VIR_ERR_OPERATION_FAILED, +_(could not parse memory balloon allocation from '%s'), reply); +goto cleanup; +} +*currmem = memMB * 1024; +ret = 1; +} else { +qemuReportError(VIR_ERR_INTERNAL_ERROR, +_(unexpected balloon information '%s'), reply); +
[libvirt] [PATCH 0/4 v3] macvtap: Support for sending port profile message for a SRIOV VF to its PF
This patch tries to fix getPhysFn in macvtap.c to get the physical function(PF) of the direct attach interface, if the interface is a SR-IOV VF. It moves some of the sriov pci device handling code from node_device_driver to src/util/pci.[ch]. This patch series implements the following 01/4 - pci: Move some pci sriov helper code out of node device driver to util/pci 02/4 - pci: Add helper functions for sriov devices 03/4 - interface: Add functions to get sriov PF/VF relationship of a net interface 04/4 - macvtap: Fix getPhysfn to get the PF of a direct attach network interface Changelog: -- v1: RFC patch introduced new functions to get PF/VF relationship of SRIOV net devices by comparing PF/VF sysfs device links. Also mentioned the existance of some related code in node_device_driver. The feedback was to move and use the node_device_driver code v2: Moves sriov get_physical_function and get_virtual_functions from node_device_driver to src/util/pci*. And reworks the rest of the patches around the new code. v3: Incorporated Stefan Berger's comments. Fixed indentation errors shown by 'make syntax-check'. Used pciReportError in src/util/pci Signed-off-by: Roopa Prabhu ropra...@cisco.com Signed-off-by: Christian Benvenuti be...@cisco.com Signed-off-by: David Wang dwa...@cisco.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/4 v3] pci: Move some pci sriov helper code out of node device driver to util/pci
From: Roopa Prabhu ropra...@cisco.com This patch moves some of the sriov related pci code from node_device driver to src/util/pci.[ch]. Some functions had to go thru name and argument list change to accommodate the move. Signed-off-by: Roopa Prabhu ropra...@cisco.com Signed-off-by: Christian Benvenuti be...@cisco.com Signed-off-by: David Wang dwa...@cisco.com --- src/Makefile.am |5 + src/conf/node_device_conf.c |1 src/conf/node_device_conf.h |7 - src/node_device/node_device_driver.h | 14 -- src/node_device/node_device_hal.c | 10 + src/node_device/node_device_linux_sysfs.c | 191 src/node_device/node_device_udev.c| 10 + src/util/pci.c| 230 + src/util/pci.h| 14 ++ 9 files changed, 265 insertions(+), 217 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index cf7c003..8fe7120 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -954,7 +954,10 @@ libvirt_driver_nodedev_la_SOURCES = $(NODE_DEVICE_DRIVER_SOURCES) libvirt_driver_nodedev_la_CFLAGS = \ -I@top_srcdir@/src/conf $(AM_CFLAGS) libvirt_driver_nodedev_la_LDFLAGS = $(AM_LDFLAGS) -libvirt_driver_nodedev_la_LIBADD = +libvirt_driver_nodedev_la_LIBADD = \ + libvirt_util.la \ + ../gnulib/lib/libgnu.la + if HAVE_HAL libvirt_driver_nodedev_la_SOURCES += $(NODE_DEVICE_DRIVER_HAL_SOURCES) libvirt_driver_nodedev_la_CFLAGS += $(HAL_CFLAGS) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index dde2921..548bbff 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -36,6 +36,7 @@ #include util.h #include buf.h #include uuid.h +#include pci.h #define VIR_FROM_THIS VIR_FROM_NODEDEV diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index cef86d4..17be031 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -82,13 +82,6 @@ enum virNodeDevPCICapFlags { VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION = (1 1), }; -struct pci_config_address { -unsigned int domain; -unsigned int bus; -unsigned int slot; -unsigned int function; -}; - typedef struct _virNodeDevCapsDef virNodeDevCapsDef; typedef virNodeDevCapsDef *virNodeDevCapsDefPtr; struct _virNodeDevCapsDef { diff --git a/src/node_device/node_device_driver.h b/src/node_device/node_device_driver.h index 08779b1..673e95b 100644 --- a/src/node_device/node_device_driver.h +++ b/src/node_device/node_device_driver.h @@ -37,10 +37,6 @@ # define LINUX_SYSFS_VPORT_CREATE_POSTFIX /vport_create # define LINUX_SYSFS_VPORT_DELETE_POSTFIX /vport_delete -# define SRIOV_FOUND 0 -# define SRIOV_NOT_FOUND 1 -# define SRIOV_ERROR -1 - # define LINUX_NEW_DEVICE_WAIT_TIME 60 # ifdef HAVE_HAL @@ -63,14 +59,6 @@ int check_fc_host_linux(union _virNodeDevCapData *d); # define check_vport_capable(d) check_vport_capable_linux(d) int check_vport_capable_linux(union _virNodeDevCapData *d); -# define get_physical_function(s,d) get_physical_function_linux(s,d) -int get_physical_function_linux(const char *sysfs_path, -union _virNodeDevCapData *d); - -# define get_virtual_functions(s,d) get_virtual_functions_linux(s,d) -int get_virtual_functions_linux(const char *sysfs_path, -union _virNodeDevCapData *d); - # define read_wwn(host, file, wwn) read_wwn_linux(host, file, wwn) int read_wwn_linux(int host, const char *file, char **wwn); @@ -78,8 +66,6 @@ int read_wwn_linux(int host, const char *file, char **wwn); # define check_fc_host(d) (-1) # define check_vport_capable(d)(-1) -# define get_physical_function(sysfs_path, d) -# define get_virtual_functions(sysfs_path, d) # define read_wwn(host, file, wwn) # endif /* __linux__ */ diff --git a/src/node_device/node_device_hal.c b/src/node_device/node_device_hal.c index 421f5ad..481be97 100644 --- a/src/node_device/node_device_hal.c +++ b/src/node_device/node_device_hal.c @@ -35,6 +35,7 @@ #include datatypes.h #include memory.h #include uuid.h +#include pci.h #include logging.h #include node_device_driver.h @@ -146,8 +147,13 @@ static int gather_pci_cap(LibHalContext *ctx, const char *udi, (void)virStrToLong_ui(p+1, p, 16, d-pci_dev.function); } -get_physical_function(sysfs_path, d); -get_virtual_functions(sysfs_path, d); +if (!pciGetPhysicalFunction(sysfs_path, d-pci_dev.physical_function)) +d-pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION; + +if (!pciGetVirtualFunctions(sysfs_path, d-pci_dev.virtual_functions, +d-pci_dev.num_virtual_functions) || +d-pci_dev.num_virtual_functions 0) +d-pci_dev.flags |=
[libvirt] [PATCH 4/4 v3] macvtap: Fix getPhysfn to get the PF of a direct attach network interface
From: Roopa Prabhu ropra...@cisco.com This patch renames getPhysfn to getPhysfnDev and adds code to get the Physical function and Virtual Function index of the direct attach linkdev (if the direct attach interface is a SRIOV VF). The idea is to send the port profile message to a PF if the direct attach interface is a SRIOV VF. Signed-off-by: Roopa Prabhu ropra...@cisco.com Signed-off-by: Christian Benvenuti be...@cisco.com Signed-off-by: David Wang dwa...@cisco.com --- src/util/macvtap.c | 23 ++- 1 files changed, 10 insertions(+), 13 deletions(-) diff --git a/src/util/macvtap.c b/src/util/macvtap.c index 0b00776..9bf7fa6 100644 --- a/src/util/macvtap.c +++ b/src/util/macvtap.c @@ -943,23 +943,20 @@ err_exit: # ifdef IFLA_VF_PORT_MAX static int -getPhysfn(const char *linkdev, - int32_t *vf, - char **physfndev) +getPhysfnDev(const char *linkdev, + int32_t *vf, + char **physfndev) { int rc = 0; -bool virtfn = false; -if (virtfn) { +if (ifaceIsVirtualFunction(linkdev)) { -/* XXX: if linkdev is SR-IOV VF, then set vf = VF index */ -/* XXX: and set linkdev = PF device */ -/* XXX: need to use get_physical_function_linux() or */ -/* XXX: something like that to get PF */ -/* XXX: device and figure out VF index */ - -rc = 1; +/* if linkdev is SR-IOV VF, then set vf = VF index */ +/* and set linkdev = PF device */ +rc = ifaceGetPhysicalFunction(linkdev, physfndev); +if (!rc) +rc = ifaceGetVirtualFunctionIndex(*physfndev, linkdev, vf); } else { /* Not SR-IOV VF: physfndev is linkdev and VF index @@ -1003,7 +1000,7 @@ doPortProfileOp8021Qbh(const char *ifname, int ifindex; int vlanid = -1; -rc = getPhysfn(ifname, vf, physfndev); +rc = getPhysfnDev(ifname, vf, physfndev); if (rc) goto err_exit; -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/4 v3] pci: Add helper functions for sriov devices
From: Roopa Prabhu ropra...@cisco.com This patch adds the following helper functions: pciDeviceIsVirtualFunction: Function to check if a pci device is a sriov VF pciGetVirtualFunctionIndex: Function to get the VF index of a sriov VF pciDeviceNetName: Function to get the network device name of a pci device pciConfigAddressCompare: Function to compare pci config addresses Signed-off-by: Roopa Prabhu ropra...@cisco.com Signed-off-by: Christian Benvenuti be...@cisco.com Signed-off-by: David Wang dwa...@cisco.com --- src/util/pci.c | 146 src/util/pci.h |8 +++ 2 files changed, 154 insertions(+), 0 deletions(-) diff --git a/src/util/pci.c b/src/util/pci.c index c3f3bb4..099c9d3 100644 --- a/src/util/pci.c +++ b/src/util/pci.c @@ -1685,6 +1685,19 @@ int pciDeviceIsAssignable(pciDevice *dev, return 1; } +/* + * returns 0 if equal and 1 if not + */ +static int +pciConfigAddressEqual(struct pci_config_address *bdf1, + struct pci_config_address *bdf2) +{ +return ((bdf1-domain == bdf2-domain) +(bdf1-bus == bdf2-bus) +(bdf1-slot == bdf2-slot) +(bdf1-function == bdf2-function)); +} + static int logStrToLong_ui(char const *s, char **end_ptr, @@ -1889,6 +1902,112 @@ out: return ret; } + +/* + * Returns 1 if vf device is a virtual function, 0 if not, -1 on error + */ +int +pciDeviceIsVirtualFunction(const char *vf_sysfs_device_link) +{ +char *vf_sysfs_physfn_link = NULL; +int ret = -1; + +if (virAsprintf(vf_sysfs_physfn_link, %s/physfn, +vf_sysfs_device_link) 0) { +virReportOOMError(); +return ret; +} + +ret = virFileExists(vf_sysfs_physfn_link); + +VIR_FREE(vf_sysfs_physfn_link); + +return ret; +} + +/* + * Returns the sriov virtual function index of vf given its pf + */ +int +pciGetVirtualFunctionIndex(const char *pf_sysfs_device_link, + const char *vf_sysfs_device_link, + int *vf_index) +{ +int ret = -1, i; +unsigned int num_virt_fns = 0; +struct pci_config_address *vf_bdf = NULL; +struct pci_config_address **virt_fns = NULL; + +if (pciGetPciConfigAddressFromSysfsDeviceLink(vf_sysfs_device_link, +vf_bdf)) +return ret; + +if (pciGetVirtualFunctions(pf_sysfs_device_link, virt_fns, +num_virt_fns)) { +pciReportError(VIR_ERR_INTERNAL_ERROR, + _(Error getting physical function's '%s' + virtual_functions), pf_sysfs_device_link); +goto out; +} + +for (i = 0; i num_virt_fns; i++) { + if (pciConfigAddressEqual(vf_bdf, virt_fns[i])) { + *vf_index = i; + ret = 0; + break; + } +} + +out: + +/* free virtual functions */ +for (i = 0; i num_virt_fns; i++) + VIR_FREE(virt_fns[i]); + +VIR_FREE(vf_bdf); + +return ret; +} + +/* + * Returns the network device name of a pci device + */ +int +pciDeviceNetName(char *device_link_sysfs_path, char **netname) +{ + char *pcidev_sysfs_net_path = NULL; + int ret = -1; + DIR *dir = NULL; + struct dirent *entry = NULL; + + if (virBuildPath(pcidev_sysfs_net_path, device_link_sysfs_path, + net) == -1) { + virReportOOMError(); + return -1; + } + + dir = opendir(pcidev_sysfs_net_path); + if (dir == NULL) + goto out; + + while ((entry = readdir(dir))) { +if (STREQ(entry-d_name, .) || +STREQ(entry-d_name, ..)) +continue; + +/* Assume a single directory entry */ +*netname = strdup(entry-d_name); +ret = 0; +break; + } + + closedir(dir); + +out: + VIR_FREE(pcidev_sysfs_net_path); + + return ret; +} #else int pciGetPhysicalFunction(const char *vf_sysfs_path, @@ -1908,4 +2027,31 @@ pciGetVirtualFunctions(const char *sysfs_path, supported on non-linux platforms)); return -1; } + +int +pciDeviceIsVirtualFunction(const char *vf_sysfs_device_link) +{ +pciReportError(VIR_ERR_INTERNAL_ERROR, _(pciDeviceIsVirtualFunction is + not supported on non-linux platforms)); +return -1; +} + +int +pciGetVirtualFunctionIndex(const char *pf_sysfs_device_link, + const char *vf_sysfs_device_link, + int *vf_index) +{ +pciReportError(VIR_ERR_INTERNAL_ERROR, _(pciGetVirtualFunctionIndex is + not supported on non-linux platforms)); +return -1; + +} + +int +pciDeviceNetName(char *device_link_sysfs_path, char **netname) +{ +pciReportError(VIR_ERR_INTERNAL_ERROR, _(pciDeviceNetName is not + supported on non-linux platforms)); +return -1; +} #endif /* __linux__ */ diff --git a/src/util/pci.h b/src/util/pci.h index fe9479e..a1600fe 100644 ---
[libvirt] [PATCH 3/4 v3] interface: Add functions to get sriov PF/VF relationship of a net interface
From: Roopa Prabhu ropra...@cisco.com This patch adds the following functions to get PF/VF relationship of an SRIOV network interface: ifaceIsVirtualFunction: Function to check if a network interface is a SRIOV VF ifaceGetVirtualFunctionIndex: Function to get VF index if a network interface is a SRIOV VF ifaceGetPhysicalFunction: Function to get the PF net interface name of a SRIOV VF net interface Signed-off-by: Roopa Prabhu ropra...@cisco.com Signed-off-by: Christian Benvenuti be...@cisco.com Signed-off-by: David Wang dwa...@cisco.com --- src/util/interface.c | 148 ++ src/util/interface.h |9 +++ 2 files changed, 157 insertions(+), 0 deletions(-) diff --git a/src/util/interface.c b/src/util/interface.c index 8b4522b..aec12f5 100644 --- a/src/util/interface.c +++ b/src/util/interface.c @@ -45,6 +45,8 @@ #include virfile.h #include memory.h #include netlink.h +#include pci.h +#include logging.h #define VIR_FROM_THIS VIR_FROM_NET @@ -1196,3 +1198,149 @@ ifaceRestoreMacAddress(const char *linkdev, return rc; } + +#ifdef __linux__ +static int +ifaceSysfsFile(char **pf_sysfs_device_link, const char *ifname, + const char *file) +{ + +if (virAsprintf(pf_sysfs_device_link, NET_SYSFS %s/%s, +ifname, file) 0) { +virReportOOMError(); +return -1; +} + +return 0; +} + +static int +ifaceSysfsDeviceFile(char **pf_sysfs_device_link, const char *ifname, + const char *file) +{ + +if (virAsprintf(pf_sysfs_device_link, NET_SYSFS %s/device/%s, +ifname, file) 0) { +virReportOOMError(); +return -1; +} + +return 0; +} + +/** + * ifaceIsVirtualFunction + * + * @ifname : name of the interface + * + * Checks if an interface is a SRIOV virtual function. + * + * Returns 1 if interface is SRIOV virtual function, 0 if not and -1 if error + * + */ +int +ifaceIsVirtualFunction(const char *ifname) +{ +char *if_sysfs_device_link = NULL; +int ret = -1; + +if (ifaceSysfsFile(if_sysfs_device_link, ifname, device)) +return ret; + +ret = pciDeviceIsVirtualFunction(if_sysfs_device_link); + +VIR_FREE(if_sysfs_device_link); + +return ret; +} + +/** + * ifaceGetVirtualFunctionIndex + * + * @pfname : name of the physical function interface name + * @vfname : name of the virtual function interface name + * @vf_index : Pointer to int. Contains vf index of interface upon successful + * return + * + * Returns 0 on success, -1 on failure + * + */ +int +ifaceGetVirtualFunctionIndex(const char *pfname, const char *vfname, + int *vf_index) +{ +char *pf_sysfs_device_link = NULL, *vf_sysfs_device_link = NULL; +int ret = -1; + +if (ifaceSysfsFile(pf_sysfs_device_link, pfname, device)) +return ret; + +if (ifaceSysfsFile(vf_sysfs_device_link, vfname, device)) { +VIR_FREE(pf_sysfs_device_link); +return ret; +} + +ret = pciGetVirtualFunctionIndex(pf_sysfs_device_link, + vf_sysfs_device_link, + vf_index); + +VIR_FREE(pf_sysfs_device_link); +VIR_FREE(vf_sysfs_device_link); + +return ret; +} + +/** + * ifaceGetPhysicalFunction + * + * @ifname : name of the physical function interface name + * @pfname : Contains sriov physical function for interface ifname + * upon successful return + * + * Returns 0 on success, -1 on failure + * + */ +int +ifaceGetPhysicalFunction(const char *ifname, char **pfname) +{ +char *physfn_sysfs_path = NULL; +int ret = -1; + +if (ifaceSysfsDeviceFile(physfn_sysfs_path, ifname, physfn)) +return ret; + +ret = pciDeviceNetName(physfn_sysfs_path, pfname); + +VIR_FREE(physfn_sysfs_path); + +return ret; +} +#else +int +ifaceIsVirtualFunction(const char *ifname) +{ +ifaceError(VIR_ERR_INTERNAL_ERROR, %s, + _(ifaceIsVirtualFunction is not supported on non-linux + platforms)); +return -1; +} + +int +ifaceGetVirtualFunctionIndex(const char *pfname, const char *vfname, + int *vf_index) +{ +ifaceError(VIR_ERR_INTERNAL_ERROR, %s, + _(ifaceGetVirtualFunctionIndex is not supported on non-linux + platforms)); +return -1; +} + +int +ifaceGetPhysicalFunction(const char *ifname, char **pfname) +{ +ifaceError(VIR_ERR_INTERNAL_ERROR, %s, + _(ifaceGetPhysicalFunction is not supported on non-linux + platforms)); +return -1; +} +#endif /* __linux__ */ diff --git a/src/util/interface.h b/src/util/interface.h index 47c0eb0..7be1444 100644 --- a/src/util/interface.h +++ b/src/util/interface.h @@ -27,6 +27,8 @@ struct nlattr; # include datatypes.h # include network.h +# define NET_SYSFS /sys/class/net/ + int ifaceGetFlags(const char *name, short *flags); int ifaceIsUp(const char *name, bool *up); @@