Re: [libvirt] [PATCH 00/21] Support NBD for tunnelled migration
Ping. May I have your attention guys? Pavel On Wed, Nov 18, 2015 at 8:12 PM, Pavel Boldin wrote: > The provided patchset implements NBD disk migration over a tunnelled > connection provided by libvirt. > > The migration source instructs QEMU to NBD mirror drives into the provided > UNIX socket. These connections and all the data are then tunnelled to the > destination using newly introduced RPC call. The migration destination > implements a driver method that connects the tunnelled stream to the QEMU's > NBD destination. > > The detailed scheme is the following: > > PREPARE > 1. Migration destination starts QEMU's NBD server listening on a UNIX > socket > using the `nbd-server-add` monitor command and tells NBD to accept > listed > disks via code added to qemuMigrationStartNBDServer that calls > introduced > qemuMonitorNBDServerStartUnix monitor function. > > PERFORM > 2. Migration source creates a UNIX socket that is later used as NBDs > destination in `drive-mirror` monitor command. > > This is implemented as a call to virNetSocketNewListenUnix from > doTunnelMigrate. > > 3. Source starts IOThread that polls on the UNIX socket, accepting every > incoming QEMU connection. > > This is done by adding a new pollfd in the poll(2) call in > qemuMigrationIOFunc that calls introduced qemuNBDTunnelAcceptAndPipe > function. > > 4. The qemuNBDTunnelAcceptAndPipe function accepts the connection and > creates > two virStream's. One is `local` that is later associated with just > accepted > connection using virFDStreamOpen. Second is `remote` that is later > tunnelled to the remote destination stream. > > The `local` stream is converted to a virFDStreamDrv stream using the > virFDStreamOpen call on the fd returned by accept(2). > > The `remote` stream is associated with a stream on the destination in > the way similar to used by PrepareTunnel3* function. That is, the > virDomainMigrateOpenTunnel function called on the destination > connection object. The virDomainMigrateOpenTunnel calls remote driver's > handler remoteDomainMigrateOpenTunnel that makes > DOMAIN_MIGRATE_OPEN_TUNNEL > call to the destination host. The code in remoteDomainMigrateOpenTunnel > ties passed virStream object to a virStream on the destination host via > remoteStreamDrv driver. The remote driver handles stream's IO by > tunnelling > data through the RPC connection. > > The qemuNBDTunnelAcceptAndPipe at last assigns both streams the same > event > callback qemuMigrationPipeEvent. Its job is to track statuses of the > streams doing IO whenever it is necessary. > > 5. Source starts the drive mirroring using the qemuMigrationDriveMirror > func. > The function instructs QEMU to mirror drives to the UNIX socket that > thread > listens on. > > Since it is necessary for the mirror driving to get into the > 'synchronized' > state, where writes go to both destinations simultaneously, before > continuing VM migration, the thread serving the connections must be > started earlier. > > 6. When the connection to a UNIX socket on the migration source is made > the DOMAIN_MIGRATE_OPEN_TUNNEL proc is called on the migration > destination. > > The handler of this code calls virDomainMigrateOpenTunnel which calls > qemuMigrationOpenNBDTunnel by the means of qemuDomainMigrateOpenTunnel. > > The qemuMigrationOpenNBDTunnel connects the stream linked to a source's > stream to the NBD's UNIX socket on the migration destination side. > > 7. The rest of the disk migration occurs semimagically: virStream* APIs > tunnel > data in both directions. This is done by qemuMigrationPipeEvent event > callback set for both streams. > > > The order of the patches is roughly the following: > > * First, the RPC machinery and remote driver's > virDrvDomainMigrateOpenTunnel > implementation are added. > > * Then, the source-side of the protocol is implemented: code listening > on a UNIX socket is added, DriveMirror is enhanced to instruct QEMU to > `drive-mirror` here and starting IOThread driving the tunneling sooner. > > * After that, the destination-side of the protocol is implemented: > the qemuMonitorNBDServerStartUnix added and qemuMigrationStartNBDServer > enhanced to call it. The qemuDomainMigrateOpenTunnel is implemented > along with qemuMigrationOpenNBDTunnel that does the real job. > > * Finally, the code blocking NBD migration for tunnelled migration is > removed. > > Pavel Boldin (21): > rpc:
[libvirt] [PATCH 16/21] qemu: migration: dest: nbd-server to UNIX sock
Modify qemuMigrationStartNBDServer so it can instruct QEMU to start NBD server binded to a local UNIX socket. Signed-off-by: Pavel Boldin --- src/qemu/qemu_migration.c | 18 +++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 43f71e9..303cd47 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1694,6 +1694,7 @@ qemuMigrationPrecreateStorage(virConnectPtr conn, static int qemuMigrationStartNBDServer(virQEMUDriverPtr driver, virDomainObjPtr vm, +bool tunnel, const char *listenAddr, size_t nmigrate_disks, const char **migrate_disks) @@ -1701,8 +1702,9 @@ qemuMigrationStartNBDServer(virQEMUDriverPtr driver, int ret = -1; qemuDomainObjPrivatePtr priv = vm->privateData; unsigned short port = 0; -char *diskAlias = NULL; +char *diskAlias = NULL, *tunnelName = NULL; size_t i; +virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); for (i = 0; i < vm->def->ndisks; i++) { virDomainDiskDefPtr disk = vm->def->disks[i]; @@ -1720,12 +1722,20 @@ qemuMigrationStartNBDServer(virQEMUDriverPtr driver, QEMU_ASYNC_JOB_MIGRATION_IN) < 0) goto cleanup; -if (!port && +if (!tunnel && !port && ((virPortAllocatorAcquire(driver->migrationPorts, &port) < 0) || (qemuMonitorNBDServerStart(priv->mon, listenAddr, port) < 0))) { goto exit_monitor; } +if (tunnel && !tunnelName && +((virAsprintf(&tunnelName, + "%s/domain-%s/qemu.nbdtunnelmigrate.src", + cfg->libDir, vm->def->name) < 0) || + (qemuMonitorNBDServerStartUnix(priv->mon, tunnelName) < 0))) { +goto exit_monitor; +} + if (qemuMonitorNBDServerAdd(priv->mon, diskAlias, true) < 0) goto exit_monitor; if (qemuDomainObjExitMonitor(driver, vm) < 0) @@ -1736,7 +1746,9 @@ qemuMigrationStartNBDServer(virQEMUDriverPtr driver, ret = 0; cleanup: +virObjectUnref(cfg); VIR_FREE(diskAlias); +VIR_FREE(tunnelName); if (ret < 0) virPortAllocatorRelease(driver->migrationPorts, port); return ret; @@ -3488,7 +3500,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, if (mig->nbd && flags & (VIR_MIGRATE_NON_SHARED_DISK | VIR_MIGRATE_NON_SHARED_INC) && virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NBD_SERVER)) { -if (qemuMigrationStartNBDServer(driver, vm, listenAddress, +if (qemuMigrationStartNBDServer(driver, vm, tunnel, listenAddress, nmigrate_disks, migrate_disks) < 0) { /* error already reported */ goto endjob; -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 02/21] driver: add virDrvDomainMigrateOpenTunnel
Add virDrvDomainMigrateOpenTunnel call that is to be implemented by the drivers in order to provide a way to open tunnel during migration. Signed-off-by: Pavel Boldin --- src/driver-hypervisor.h | 8 1 file changed, 8 insertions(+) diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h index ae2ec4d..30a7446 100644 --- a/src/driver-hypervisor.h +++ b/src/driver-hypervisor.h @@ -1212,6 +1212,13 @@ typedef int const char *password, unsigned int flags); +typedef int +(*virDrvDomainMigrateOpenTunnel)(virConnectPtr dconn, + virStreamPtr st, + unsigned char uuid[VIR_UUID_BUFLEN], + unsigned int flags); + + typedef struct _virHypervisorDriver virHypervisorDriver; typedef virHypervisorDriver *virHypervisorDriverPtr; @@ -1443,6 +1450,7 @@ struct _virHypervisorDriver { virDrvDomainGetFSInfo domainGetFSInfo; virDrvDomainInterfaceAddresses domainInterfaceAddresses; virDrvDomainSetUserPassword domainSetUserPassword; +virDrvDomainMigrateOpenTunnel domainMigrateOpenTunnel; }; -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 00/21] Support NBD for tunnelled migration
The provided patchset implements NBD disk migration over a tunnelled connection provided by libvirt. The migration source instructs QEMU to NBD mirror drives into the provided UNIX socket. These connections and all the data are then tunnelled to the destination using newly introduced RPC call. The migration destination implements a driver method that connects the tunnelled stream to the QEMU's NBD destination. The detailed scheme is the following: PREPARE 1. Migration destination starts QEMU's NBD server listening on a UNIX socket using the `nbd-server-add` monitor command and tells NBD to accept listed disks via code added to qemuMigrationStartNBDServer that calls introduced qemuMonitorNBDServerStartUnix monitor function. PERFORM 2. Migration source creates a UNIX socket that is later used as NBDs destination in `drive-mirror` monitor command. This is implemented as a call to virNetSocketNewListenUnix from doTunnelMigrate. 3. Source starts IOThread that polls on the UNIX socket, accepting every incoming QEMU connection. This is done by adding a new pollfd in the poll(2) call in qemuMigrationIOFunc that calls introduced qemuNBDTunnelAcceptAndPipe function. 4. The qemuNBDTunnelAcceptAndPipe function accepts the connection and creates two virStream's. One is `local` that is later associated with just accepted connection using virFDStreamOpen. Second is `remote` that is later tunnelled to the remote destination stream. The `local` stream is converted to a virFDStreamDrv stream using the virFDStreamOpen call on the fd returned by accept(2). The `remote` stream is associated with a stream on the destination in the way similar to used by PrepareTunnel3* function. That is, the virDomainMigrateOpenTunnel function called on the destination connection object. The virDomainMigrateOpenTunnel calls remote driver's handler remoteDomainMigrateOpenTunnel that makes DOMAIN_MIGRATE_OPEN_TUNNEL call to the destination host. The code in remoteDomainMigrateOpenTunnel ties passed virStream object to a virStream on the destination host via remoteStreamDrv driver. The remote driver handles stream's IO by tunnelling data through the RPC connection. The qemuNBDTunnelAcceptAndPipe at last assigns both streams the same event callback qemuMigrationPipeEvent. Its job is to track statuses of the streams doing IO whenever it is necessary. 5. Source starts the drive mirroring using the qemuMigrationDriveMirror func. The function instructs QEMU to mirror drives to the UNIX socket that thread listens on. Since it is necessary for the mirror driving to get into the 'synchronized' state, where writes go to both destinations simultaneously, before continuing VM migration, the thread serving the connections must be started earlier. 6. When the connection to a UNIX socket on the migration source is made the DOMAIN_MIGRATE_OPEN_TUNNEL proc is called on the migration destination. The handler of this code calls virDomainMigrateOpenTunnel which calls qemuMigrationOpenNBDTunnel by the means of qemuDomainMigrateOpenTunnel. The qemuMigrationOpenNBDTunnel connects the stream linked to a source's stream to the NBD's UNIX socket on the migration destination side. 7. The rest of the disk migration occurs semimagically: virStream* APIs tunnel data in both directions. This is done by qemuMigrationPipeEvent event callback set for both streams. The order of the patches is roughly the following: * First, the RPC machinery and remote driver's virDrvDomainMigrateOpenTunnel implementation are added. * Then, the source-side of the protocol is implemented: code listening on a UNIX socket is added, DriveMirror is enhanced to instruct QEMU to `drive-mirror` here and starting IOThread driving the tunneling sooner. * After that, the destination-side of the protocol is implemented: the qemuMonitorNBDServerStartUnix added and qemuMigrationStartNBDServer enhanced to call it. The qemuDomainMigrateOpenTunnel is implemented along with qemuMigrationOpenNBDTunnel that does the real job. * Finally, the code blocking NBD migration for tunnelled migration is removed. Pavel Boldin (21): rpc: add DOMAIN_MIGRATE_OPEN_TUNNEL proc driver: add virDrvDomainMigrateOpenTunnel remote_driver: introduce virRemoteClientNew remote_driver: add remoteDomainMigrateOpenTunnel domain: add virDomainMigrateOpenTunnel domain: add virDomainMigrateTunnelFlags remote: impl remoteDispatchDomainMigrateOpenTunnel qemu: migration: src: add nbd tunnel socket data qemu: migration: src: nbdtunnel unix socket qemu: migration: src: qemu `drive-mirror` to UNIX qemu: migration: src: qemuSock for running thread qemu: migration: src: add NBD unixSock to iothread qemu: migration: src: qemuNBDTunnelAcceptAndPipe q
[libvirt] [PATCH 20/21] qemu: migration: allow NBD tunneling migration
Now that all the pieces are in their places finally allow NBD in tunnelled migration. Signed-off-by: Pavel Boldin --- src/qemu/qemu_migration.c | 16 ++-- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 27c1acb..9520e34 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3041,13 +3041,6 @@ qemuMigrationBeginPhase(virQEMUDriverPtr driver, goto cleanup; } } - -if (flags & VIR_MIGRATE_TUNNELLED) { -virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("Selecting disks to migrate is not " - "implemented for tunnelled migration")); -goto cleanup; -} } else { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", _("qemu does not support drive-mirror command")); @@ -3056,13 +3049,8 @@ qemuMigrationBeginPhase(virQEMUDriverPtr driver, } if (has_drive_mirror) { -/* TODO support NBD for TUNNELLED migration */ -if (flags & VIR_MIGRATE_TUNNELLED) { -VIR_WARN("NBD in tunnelled migration is currently not supported"); -} else { -cookieFlags |= QEMU_MIGRATION_COOKIE_NBD; -priv->nbdPort = 0; -} +cookieFlags |= QEMU_MIGRATION_COOKIE_NBD; +priv->nbdPort = 0; } } -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 11/21] qemu: migration: src: qemuSock for running thread
Tunnelled drive mirroring requires an active thread to accept incoming connections from the QEMU and pumping them to the remote host through the tunnel. For this, we need to split thread's QEMU socket initialization from the start of the thread and introduce qemuMigrationSetQEMUSocket to specify it later. Signed-off-by: Pavel Boldin --- src/qemu/qemu_migration.c | 93 ++- 1 file changed, 59 insertions(+), 34 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index d95cd66..61e78c5 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3991,14 +3991,15 @@ typedef struct _qemuMigrationIOThread qemuMigrationIOThread; typedef qemuMigrationIOThread *qemuMigrationIOThreadPtr; struct _qemuMigrationIOThread { virThread thread; -virStreamPtr st; -int sock; +virStreamPtr qemuStream; +int qemuSock; virError err; int wakeupRecvFD; int wakeupSendFD; }; -static void qemuMigrationIOFunc(void *arg) +static void +qemuMigrationIOFunc(void *arg) { qemuMigrationIOThreadPtr data = arg; char *buffer = NULL; @@ -4006,21 +4007,18 @@ static void qemuMigrationIOFunc(void *arg) int timeout = -1; virErrorPtr err = NULL; -VIR_DEBUG("Running migration tunnel; stream=%p, sock=%d", - data->st, data->sock); +VIR_DEBUG("Running migration tunnel; qemuStream=%p", data->qemuStream); if (VIR_ALLOC_N(buffer, TUNNEL_SEND_BUF_SIZE) < 0) goto abrt; -fds[0].fd = data->sock; -fds[1].fd = data->wakeupRecvFD; +fds[0].fd = data->wakeupRecvFD; +fds[1].fd = -1; +fds[0].events = fds[1].events = POLLIN; for (;;) { int ret; -fds[0].events = fds[1].events = POLLIN; -fds[0].revents = fds[1].revents = 0; - ret = poll(fds, ARRAY_CARDINALITY(fds), timeout); if (ret < 0) { @@ -4040,30 +4038,36 @@ static void qemuMigrationIOFunc(void *arg) break; } -if (fds[1].revents & (POLLIN | POLLERR | POLLHUP)) { -char stop = 0; +if (fds[0].revents & (POLLIN | POLLERR | POLLHUP)) { +char action = 0; -if (saferead(data->wakeupRecvFD, &stop, 1) != 1) { +if (saferead(data->wakeupRecvFD, &action, 1) != 1) { virReportSystemError(errno, "%s", _("failed to read from wakeup fd")); goto abrt; } -VIR_DEBUG("Migration tunnel was asked to %s", - stop ? "abort" : "finish"); -if (stop) { -goto abrt; -} else { -timeout = 0; +VIR_DEBUG("Migration tunnel was asked to %c", action); +switch (action) { +case 's': +goto abrt; +break; +case 'f': +timeout = 0; +break; +case 'u': +fds[1].fd = data->qemuSock; +VIR_DEBUG("qemuSock set %d", data->qemuSock); +break; } } -if (fds[0].revents & (POLLIN | POLLERR | POLLHUP)) { +if (fds[1].revents & (POLLIN | POLLERR | POLLHUP)) { int nbytes; -nbytes = saferead(data->sock, buffer, TUNNEL_SEND_BUF_SIZE); +nbytes = saferead(data->qemuSock, buffer, TUNNEL_SEND_BUF_SIZE); if (nbytes > 0) { -if (virStreamSend(data->st, buffer, nbytes) < 0) +if (virStreamSend(data->qemuStream, buffer, nbytes) < 0) goto error; } else if (nbytes < 0) { virReportSystemError(errno, "%s", @@ -4076,10 +4080,9 @@ static void qemuMigrationIOFunc(void *arg) } } -if (virStreamFinish(data->st) < 0) -goto error; +virStreamFinish(data->qemuStream); -VIR_FORCE_CLOSE(data->sock); +VIR_FORCE_CLOSE(data->qemuSock); VIR_FREE(buffer); return; @@ -4090,7 +4093,7 @@ static void qemuMigrationIOFunc(void *arg) virFreeError(err); err = NULL; } -virStreamAbort(data->st); +virStreamAbort(data->qemuStream); if (err) { virSetError(err); virFreeError(err); @@ -4099,7 +4102,7 @@ static void qemuMigrationIOFunc(void *arg) error: /* Let the source qemu know that the transfer cant continue anymore. * Don't copy the error for EPIPE as destination has the actual error. */ -VIR_FORCE_CLOSE(data->sock); +VIR_FORCE_CLOSE(data->qemuSock); if (!virLastErrorIsSystemErrno(EPIPE)) virCopyLastError(&data->err)
[libvirt] [PATCH 09/21] qemu: migration: src: nbdtunnel unix socket
Create a UNIX socket that will be a target for outgoing NBD connection from the QEMU side. Signed-off-by: Pavel Boldin --- src/qemu/qemu_migration.c | 24 +++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index fb2a216..d587c56 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -4597,7 +4597,7 @@ static int doTunnelMigrate(virQEMUDriverPtr driver, size_t nmigrate_disks, const char **migrate_disks) { -virNetSocketPtr sock = NULL; +virNetSocketPtr sock = NULL, nbdSock = NULL; int ret = -1; qemuMigrationSpec spec; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); @@ -4613,6 +4613,23 @@ static int doTunnelMigrate(virQEMUDriverPtr driver, spec.fwdType = MIGRATION_FWD_STREAM; spec.fwd.stream = st; +if (nmigrate_disks) { +spec.nbd_tunnel_unix_socket.sock = -1; +spec.nbd_tunnel_unix_socket.file = NULL; + +if (virAsprintf(&spec.nbd_tunnel_unix_socket.file, +"%s/domain-%s/qemu.nbdtunnelmigrate.src", +cfg->libDir, vm->def->name) < 0) +goto cleanup; + +if (virNetSocketNewListenUNIX(spec.nbd_tunnel_unix_socket.file, 0700, + cfg->user, cfg->group, + &nbdSock) < 0 || +virNetSocketListen(nbdSock, 1) < 0) +goto cleanup; + +spec.nbd_tunnel_unix_socket.sock = virNetSocketGetFD(nbdSock); +} spec.destType = MIGRATION_DEST_FD; spec.dest.fd.qemu = -1; @@ -4643,6 +4660,11 @@ static int doTunnelMigrate(virQEMUDriverPtr driver, VIR_FREE(spec.dest.unix_socket.file); } +if (nmigrate_disks) { +virObjectUnref(nbdSock); +VIR_FREE(spec.nbd_tunnel_unix_socket.file); +} + virObjectUnref(cfg); return ret; } -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 15/21] qemu: monitor: add qemuMonitorNBDServerStartUnix
Add qemuMonitorNBDServerStartUnix used to instruct QEMU to connect to a UNIX socket as a NBD drive mirror destination. Signed-off-by: Pavel Boldin --- src/qemu/qemu_monitor.c | 12 src/qemu/qemu_monitor.h | 2 ++ src/qemu/qemu_monitor_json.c | 35 +++ src/qemu/qemu_monitor_json.h | 2 ++ 4 files changed, 51 insertions(+) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 49d4aa2..9c8e0fe 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3594,6 +3594,18 @@ qemuMonitorNBDServerStart(qemuMonitorPtr mon, int +qemuMonitorNBDServerStartUnix(qemuMonitorPtr mon, + const char *file) +{ +VIR_DEBUG("file=%s", file); + +QEMU_CHECK_MONITOR_JSON(mon); + +return qemuMonitorJSONNBDServerStartUnix(mon, file); +} + + +int qemuMonitorNBDServerAdd(qemuMonitorPtr mon, const char *deviceID, bool writable) diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 2ce3958..e94ac93 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -873,6 +873,8 @@ char *qemuMonitorGetTargetArch(qemuMonitorPtr mon); int qemuMonitorNBDServerStart(qemuMonitorPtr mon, const char *host, unsigned int port); +int qemuMonitorNBDServerStartUnix(qemuMonitorPtr mon, + const char *path); int qemuMonitorNBDServerAdd(qemuMonitorPtr mon, const char *deviceID, bool writable); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index b39b29b..ef162a4 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -5749,6 +5749,41 @@ qemuMonitorJSONNBDServerStart(qemuMonitorPtr mon, } int +qemuMonitorJSONNBDServerStartUnix(qemuMonitorPtr mon, + const char *file) +{ +int ret = -1; +virJSONValuePtr cmd = NULL; +virJSONValuePtr reply = NULL; +virJSONValuePtr addr = NULL; + +if (!(addr = qemuMonitorJSONBuildUnixSocketAddress(file))) +return ret; + +if (!(cmd = qemuMonitorJSONMakeCommand("nbd-server-start", + "a:addr", addr, + NULL))) +goto cleanup; + +/* From now on, @addr is part of @cmd */ +addr = NULL; + +if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) +goto cleanup; + +if (qemuMonitorJSONCheckError(cmd, reply) < 0) +goto cleanup; + +ret = 0; + + cleanup: +virJSONValueFree(reply); +virJSONValueFree(cmd); +virJSONValueFree(addr); +return ret; +} + +int qemuMonitorJSONNBDServerAdd(qemuMonitorPtr mon, const char *deviceID, bool writable) diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 120bd93..28e17c0 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -441,6 +441,8 @@ char *qemuMonitorJSONGetTargetArch(qemuMonitorPtr mon); int qemuMonitorJSONNBDServerStart(qemuMonitorPtr mon, const char *host, unsigned int port); +int qemuMonitorJSONNBDServerStartUnix(qemuMonitorPtr mon, + const char *path); int qemuMonitorJSONNBDServerAdd(qemuMonitorPtr mon, const char *deviceID, bool writable); -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 01/21] rpc: add DOMAIN_MIGRATE_OPEN_TUNNEL proc
Add REMOTE_PROC_DOMAIN_MIGRATE_OPEN_TUNNEL remote call, args and rets. Signed-off-by: Pavel Boldin --- daemon/remote.c | 12 src/remote/remote_protocol.x | 19 ++- src/remote_protocol-structs | 8 3 files changed, 38 insertions(+), 1 deletion(-) diff --git a/daemon/remote.c b/daemon/remote.c index 3a3eb09..237124d 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -6661,6 +6661,18 @@ remoteDispatchDomainInterfaceAddresses(virNetServerPtr server ATTRIBUTE_UNUSED, } +static int +remoteDispatchDomainMigrateOpenTunnel(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr ATTRIBUTE_UNUSED, + remote_domain_migrate_open_tunnel_args *args ATTRIBUTE_UNUSED, + remote_domain_migrate_open_tunnel_ret *ret ATTRIBUTE_UNUSED) +{ +return -1; +} + + /*- Helpers. -*/ /* get_nonnull_domain and get_nonnull_network turn an on-wire diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 80f4a8b..3d8702f 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -3240,6 +3240,15 @@ struct remote_domain_rename_ret { int retcode; }; +struct remote_domain_migrate_open_tunnel_args { +remote_uuid uuid; +unsigned int flags; +}; + +struct remote_domain_migrate_open_tunnel_ret { +int retcode; +}; + /*- Protocol. -*/ /* Define the program number, protocol version and procedure numbers here. */ @@ -5712,5 +5721,13 @@ enum remote_procedure { * @acl: domain:write * @acl: domain:save */ -REMOTE_PROC_DOMAIN_RENAME = 358 +REMOTE_PROC_DOMAIN_RENAME = 358, + +/** + * @generate: none + * @acl: domain:migrate + * @acl: domain:start + * @acl: domain:write + */ +REMOTE_PROC_DOMAIN_MIGRATE_OPEN_TUNNEL = 359 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index ff99c00..b065576 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -2692,6 +2692,13 @@ struct remote_domain_rename_args { struct remote_domain_rename_ret { intretcode; }; +struct remote_domain_migrate_open_tunnel_args { +remote_uuiduuid; +u_int flags; +}; +struct remote_domain_migrate_open_tunnel_ret { +intretcode; +}; enum remote_procedure { REMOTE_PROC_CONNECT_OPEN = 1, REMOTE_PROC_CONNECT_CLOSE = 2, @@ -3051,4 +3058,5 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_DEL_IOTHREAD = 356, REMOTE_PROC_DOMAIN_SET_USER_PASSWORD = 357, REMOTE_PROC_DOMAIN_RENAME = 358, +REMOTE_PROC_DOMAIN_MIGRATE_OPEN_TUNNEL = 359, }; -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 07/21] remote: impl remoteDispatchDomainMigrateOpenTunnel
Implement remoteDispatchDomainMigrateOpenTunnel. Signed-off-by: Pavel Boldin --- daemon/remote.c | 50 -- 1 file changed, 44 insertions(+), 6 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 237124d..3c6803e 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -6663,13 +6663,51 @@ remoteDispatchDomainInterfaceAddresses(virNetServerPtr server ATTRIBUTE_UNUSED, static int remoteDispatchDomainMigrateOpenTunnel(virNetServerPtr server ATTRIBUTE_UNUSED, - virNetServerClientPtr client ATTRIBUTE_UNUSED, - virNetMessagePtr msg ATTRIBUTE_UNUSED, - virNetMessageErrorPtr rerr ATTRIBUTE_UNUSED, - remote_domain_migrate_open_tunnel_args *args ATTRIBUTE_UNUSED, - remote_domain_migrate_open_tunnel_ret *ret ATTRIBUTE_UNUSED) + virNetServerClientPtr client, + virNetMessagePtr msg, + virNetMessageErrorPtr rerr, + remote_domain_migrate_open_tunnel_args *args, + remote_domain_migrate_open_tunnel_ret *ret) { -return -1; +int rv = -1; +struct daemonClientPrivate *priv = +virNetServerClientGetPrivateData(client); +virStreamPtr st = NULL; +daemonClientStreamPtr stream = NULL; + +if (!priv->conn) { +virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); +goto cleanup; +} + +if (!(st = virStreamNew(priv->conn, VIR_STREAM_NONBLOCK)) || +!(stream = daemonCreateClientStream(client, st, remoteProgram, +&msg->header))) +goto cleanup; + +ret->retcode = virDomainMigrateOpenTunnel(priv->conn, st, + (unsigned char *)args->uuid, + args->flags); + +if (ret->retcode < 0) +goto cleanup; + +if (daemonAddClientStream(client, stream, true) < 0) +goto cleanup; + +rv = 0; + + cleanup: +if (rv < 0) { +virNetMessageSaveError(rerr); +if (stream) { +virStreamAbort(st); +daemonFreeClientStream(client, stream); +} else { +virObjectUnref(st); +} +} +return rv; } -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 14/21] qemu: migration: src: stream piping
Add and use qemuMigrationPipeEvent piped streams' event handler. It sets the appropriate event flags for each of the stream and pumps the pipe using qemuMigrationPipeIO whenever there is a data at any end. Signed-off-by: Pavel Boldin --- src/qemu/qemu_migration.c | 85 ++- 1 file changed, 84 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 0f35c13..43f71e9 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -4010,8 +4010,28 @@ struct _qemuMigrationPipe { qemuMigrationIOThreadPtr data; virStreamPtr local; virStreamPtr remote; + +int local_flags : 4; +int remote_flags : 4; +char buffer[TUNNEL_SEND_BUF_SIZE]; }; +static int +qemuMigrationPipeIO(virStreamPtr from, virStreamPtr to, char *buffer) +{ +int done, got, offset = 0; +got = virStreamRecv(from, buffer, TUNNEL_SEND_BUF_SIZE); + +while (offset < got) { +done = virStreamSend(to, buffer + offset, got - offset); +if (done < 0) +break; +offset += done; +} + +return got; +} + static void qemuMigrationPipeClose(qemuMigrationPipePtr pipe, bool abort) { @@ -4030,6 +4050,55 @@ qemuMigrationPipeClose(qemuMigrationPipePtr pipe, bool abort) virObjectUnref(pipe->remote); } +static void +qemuMigrationPipeEvent(virStreamPtr stream, int events, void *opaque) +{ +qemuMigrationPipePtr pipe = opaque; + +if (stream == pipe->remote) +pipe->remote_flags |= events; +if (stream == pipe->local) +pipe->local_flags |= events; + +VIR_DEBUG("remote = %p, remote_flags = %x, local = %p, local_flags = %x", + pipe->remote, pipe->remote_flags, + pipe->local, pipe->local_flags); + +if (events & (VIR_STREAM_EVENT_ERROR | VIR_STREAM_EVENT_HANGUP)) { +char dummy; +virStreamRecv(stream, &dummy, 1); + abrt: +virCopyLastError(&pipe->data->err); +qemuMigrationPipeClose(pipe, true); +if (safewrite(pipe->data->wakeupSendFD, "c", 1) != 1) { +virReportSystemError(errno, "%s", + _("failed to stop migration tunnel")); +} +return; +} + +if ((pipe->remote_flags & VIR_STREAM_EVENT_READABLE) && +(pipe->local_flags & VIR_STREAM_EVENT_WRITABLE)) { + +if (qemuMigrationPipeIO(pipe->remote, pipe->local, pipe->buffer) == -1) +goto abrt; + +pipe->remote_flags &= ~VIR_STREAM_EVENT_READABLE; +pipe->local_flags &= ~VIR_STREAM_EVENT_WRITABLE; +} + +if ((pipe->local_flags & VIR_STREAM_EVENT_READABLE) && +(pipe->remote_flags & VIR_STREAM_EVENT_WRITABLE)) { + +if (qemuMigrationPipeIO(pipe->local, pipe->remote, pipe->buffer) == -1) +goto abrt; + +pipe->local_flags &= ~VIR_STREAM_EVENT_READABLE; +pipe->remote_flags &= ~VIR_STREAM_EVENT_WRITABLE; +} +} + + static qemuMigrationPipePtr qemuMigrationPipeCreate(virStreamPtr local, virStreamPtr remote) { @@ -4041,6 +4110,20 @@ qemuMigrationPipeCreate(virStreamPtr local, virStreamPtr remote) pipe->local = local; pipe->remote = remote; +if (virStreamEventAddCallback(local, + VIR_STREAM_EVENT_READABLE | + VIR_STREAM_EVENT_WRITABLE, + qemuMigrationPipeEvent, + pipe, NULL) < 0) +goto error; + +if (virStreamEventAddCallback(remote, + VIR_STREAM_EVENT_READABLE | + VIR_STREAM_EVENT_WRITABLE, + qemuMigrationPipeEvent, + pipe, NULL) < 0) +goto error; + return pipe; error: @@ -4230,7 +4313,7 @@ qemuMigrationIOFunc(void *arg) /* Let the source qemu know that the transfer cant continue anymore. * Don't copy the error for EPIPE as destination has the actual error. */ VIR_FORCE_CLOSE(data->qemuSock); -if (!virLastErrorIsSystemErrno(EPIPE)) +if (data->err.code == VIR_ERR_OK && !virLastErrorIsSystemErrno(EPIPE)) virCopyLastError(&data->err); virResetLastError(); VIR_FREE(buffer); -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 04/21] remote_driver: add remoteDomainMigrateOpenTunnel
Add remoteDomainMigrateOpenTunnel that ties passed stream to the network stream and then makes the appropriate remote call. Signed-off-by: Pavel Boldin --- src/remote/remote_driver.c | 43 +++ 1 file changed, 43 insertions(+) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index b72cb86..f6571a9 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -8087,6 +8087,48 @@ remoteDomainRename(virDomainPtr dom, const char *new_name, unsigned int flags) } +static int +remoteDomainMigrateOpenTunnel(virConnectPtr dconn, + virStreamPtr st, + unsigned char uuid[VIR_UUID_BUFLEN], + unsigned int flags) +{ +struct private_data *priv = dconn->privateData; +int rv = -1; +remote_domain_migrate_open_tunnel_args args; +remote_domain_migrate_open_tunnel_ret ret; +virNetClientStreamPtr netst; + +remoteDriverLock(priv); + +memset(&args, 0, sizeof(args)); +memset(&ret, 0, sizeof(ret)); + +netst = virRemoteClientOpen(st, priv, +REMOTE_PROC_DOMAIN_MIGRATE_OPEN_TUNNEL); + +if (netst == NULL) +goto done; + +memcpy(args.uuid, uuid, VIR_UUID_BUFLEN); +args.flags = flags; + +if (call(dconn, priv, 0, REMOTE_PROC_DOMAIN_MIGRATE_OPEN_TUNNEL, + (xdrproc_t) xdr_remote_domain_migrate_open_tunnel_args, (char *) &args, + (xdrproc_t) xdr_remote_domain_migrate_open_tunnel_ret, (char *) &ret) == -1) { +virNetClientRemoveStream(priv->client, netst); +virObjectUnref(netst); +goto done; +} + +rv = ret.retcode; + + done: +remoteDriverUnlock(priv); +return rv; +} + + /* get_nonnull_domain and get_nonnull_network turn an on-wire * (name, uuid) pair into virDomainPtr or virNetworkPtr object. * These can return NULL if underlying memory allocations fail, @@ -8437,6 +8479,7 @@ static virHypervisorDriver hypervisor_driver = { .domainInterfaceAddresses = remoteDomainInterfaceAddresses, /* 1.2.14 */ .domainSetUserPassword = remoteDomainSetUserPassword, /* 1.2.16 */ .domainRename = remoteDomainRename, /* 1.2.19 */ +.domainMigrateOpenTunnel = remoteDomainMigrateOpenTunnel, /* 1.2.XX */ }; static virNetworkDriver network_driver = { -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 18/21] qemu: driver: add qemuDomainMigrateOpenTunnel
Add domainMigrateOpenTunnel handler for QEMU driver. Signed-off-by: Pavel Boldin --- src/qemu/qemu_driver.c | 24 1 file changed, 24 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 92a9961..ad9a6a0 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -20064,6 +20064,29 @@ static int qemuDomainRename(virDomainPtr dom, goto endjob; } +static int qemuDomainMigrateOpenTunnel(virConnectPtr dconn, + virStreamPtr st, + unsigned char uuid[VIR_UUID_BUFLEN], + unsigned int flags) +{ +virQEMUDriverPtr driver = dconn->privateData; +int ret = -1; +virDomainObjPtr vm; + +virCheckFlags(VIR_MIGRATE_TUNNEL_NBD, -1); + +vm = virDomainObjListFindByUUIDRef(driver->domains, uuid); + +if (virDomainMigrateOpenTunnelEnsureACL(dconn, vm->def) < 0) +goto cleanup; + +ret = qemuMigrationOpenTunnel(driver, dconn, st, vm->def, flags); + + cleanup: +virDomainObjEndAPI(&vm); +return ret; +} + static virHypervisorDriver qemuHypervisorDriver = { .name = QEMU_DRIVER_NAME, .connectOpen = qemuConnectOpen, /* 0.2.0 */ @@ -20272,6 +20295,7 @@ static virHypervisorDriver qemuHypervisorDriver = { .domainInterfaceAddresses = qemuDomainInterfaceAddresses, /* 1.2.14 */ .domainSetUserPassword = qemuDomainSetUserPassword, /* 1.2.16 */ .domainRename = qemuDomainRename, /* 1.2.19 */ +.domainMigrateOpenTunnel = qemuDomainMigrateOpenTunnel, /* 1.2.XX */ }; -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 19/21] qemu: migration: dest: qemuMigrationOpenNBDTunnel
Add qemuMigrationOpenNBDTunnel that connects a remote stream to the local NBD UNIX socket. Signed-off-by: Pavel Boldin --- src/qemu/qemu_migration.c | 29 + 1 file changed, 29 insertions(+) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 4708387..27c1acb 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3605,6 +3605,32 @@ qemuMigrationPrepareTunnel(virQEMUDriverPtr driver, } +static int +qemuMigrationOpenNBDTunnel(virQEMUDriverPtr driver, + virStreamPtr st, + const char *name) +{ +char *tunnelName = NULL; +int ret = -1; +virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + +if (virAsprintf(&tunnelName, +"%s/domain-%s/qemu.nbdtunnelmigrate.src", +cfg->libDir, name) < 0) +goto cleanup; + +if (virFDStreamConnectUNIX(st, tunnelName, false) < 0) +goto cleanup; + +ret = 0; + + cleanup: +VIR_FREE(tunnelName); +virObjectUnref(cfg); +return ret; +} + + int qemuMigrationOpenTunnel(virQEMUDriverPtr driver, virConnectPtr dconn, @@ -3621,6 +3647,9 @@ qemuMigrationOpenTunnel(virQEMUDriverPtr driver, return -1; } +if (flags & VIR_MIGRATE_TUNNEL_NBD) +return qemuMigrationOpenNBDTunnel(driver, st, def->name); + return 0; } -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 21/21] apparmor: fix tunnelmigrate permissions
Signed-off-by: Pavel Boldin --- src/security/virt-aa-helper.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 5de56e5..87af98f 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -1367,9 +1367,9 @@ main(int argc, char **argv) LOCALSTATEDIR, ctl->def->name); virBufferAsprintf(&buf, " \"/run/libvirt/**/%s.pid\" rwk,\n", ctl->def->name); -virBufferAsprintf(&buf, " \"%s/run/libvirt/**/*.tunnelmigrate.dest.%s\" rw,\n", +virBufferAsprintf(&buf, " \"%s/lib/libvirt/qemu/domain-%s/*tunnelmigrate.src\" rw,\n", LOCALSTATEDIR, ctl->def->name); -virBufferAsprintf(&buf, " \"/run/libvirt/**/*.tunnelmigrate.dest.%s\" rw,\n", +virBufferAsprintf(&buf, " \"/lib/libvirt/qemu/domain-%s/*tunnelmigrate.src\" rw,\n", ctl->def->name); } if (ctl->files) -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 17/21] qemu: migration: dest: qemuMigrationOpenTunnel
Introduce an auxiliary handler domainMigrateOpenTunnel for QEMU. Signed-off-by: Pavel Boldin --- src/qemu/qemu_migration.c | 19 +++ src/qemu/qemu_migration.h | 6 ++ 2 files changed, 25 insertions(+) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 303cd47..4708387 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3605,6 +3605,25 @@ qemuMigrationPrepareTunnel(virQEMUDriverPtr driver, } +int +qemuMigrationOpenTunnel(virQEMUDriverPtr driver, +virConnectPtr dconn, +virStreamPtr st, +virDomainDefPtr def, +unsigned long flags) +{ +VIR_DEBUG("driver=%p, dconn=%p, st=%p, def=%p, flags=%lx", + driver, dconn, st, def, flags); + +if (st == NULL) { +virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("opening a tunnel requested but NULL stream passed")); +return -1; +} + +return 0; +} + static virURIPtr qemuMigrationParseURI(const char *uri, bool *wellFormed) { diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h index 8175f4b..f91791e 100644 --- a/src/qemu/qemu_migration.h +++ b/src/qemu/qemu_migration.h @@ -121,6 +121,12 @@ int qemuMigrationPrepareTunnel(virQEMUDriverPtr driver, const char *origname, unsigned long flags); +int qemuMigrationOpenTunnel(virQEMUDriverPtr driver, +virConnectPtr dconn, +virStreamPtr st, +virDomainDefPtr def, +unsigned long flags); + int qemuMigrationPrepareDirect(virQEMUDriverPtr driver, virConnectPtr dconn, const char *cookiein, -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 13/21] qemu: migration: src: qemuNBDTunnelAcceptAndPipe
Add qemuNBDTunnelAcceptAndPipe function that is called to handle POLLIN on the UNIX socket connection from the QEMU's NBD server. The function creates a pipe of a remote stream connected to the QEMU NBD Unix socket on destination and a local stream connected to the incoming connection from the source QEMU's NBD. Signed-off-by: Pavel Boldin --- src/qemu/qemu_migration.c | 134 +- 1 file changed, 132 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 0682fd8..0f35c13 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3987,6 +3987,9 @@ struct _qemuMigrationSpec { #define TUNNEL_SEND_BUF_SIZE 65536 +typedef struct _qemuMigrationPipe qemuMigrationPipe; +typedef qemuMigrationPipe *qemuMigrationPipePtr; + typedef struct _qemuMigrationIOThread qemuMigrationIOThread; typedef qemuMigrationIOThread *qemuMigrationIOThreadPtr; struct _qemuMigrationIOThread { @@ -3997,9 +4000,124 @@ struct _qemuMigrationIOThread { virError err; int wakeupRecvFD; int wakeupSendFD; +qemuMigrationPipePtr pipes; +virConnectPtr dconn; +unsigned char uuid[VIR_UUID_BUFLEN]; +}; + +struct _qemuMigrationPipe { +qemuMigrationPipePtr next; +qemuMigrationIOThreadPtr data; +virStreamPtr local; +virStreamPtr remote; }; static void +qemuMigrationPipeClose(qemuMigrationPipePtr pipe, bool abort) +{ +virStreamEventUpdateCallback(pipe->local, 0); +virStreamEventUpdateCallback(pipe->remote, 0); + +if (abort) { +virStreamAbort(pipe->local); +virStreamAbort(pipe->remote); +} else { +virStreamFinish(pipe->local); +virStreamFinish(pipe->remote); +} + +virObjectUnref(pipe->local); +virObjectUnref(pipe->remote); +} + +static qemuMigrationPipePtr +qemuMigrationPipeCreate(virStreamPtr local, virStreamPtr remote) +{ +qemuMigrationPipePtr pipe = NULL; + +if (VIR_ALLOC(pipe) < 0) +goto error; + +pipe->local = local; +pipe->remote = remote; + +return pipe; + + error: +virStreamEventRemoveCallback(local); +virStreamEventRemoveCallback(remote); +VIR_FREE(pipe); +return NULL; +} + + +static int +qemuNBDTunnelAcceptAndPipe(qemuMigrationIOThreadPtr data) +{ +int fd, ret; +virStreamPtr local = NULL, remote = NULL; +qemuMigrationPipePtr pipe = NULL; + +while ((fd = accept(data->unixSock, NULL, NULL)) < 0) { +if (errno == EAGAIN || errno == EINTR) +continue; +virReportSystemError( +errno, "%s", _("failed to accept connection from qemu")); +goto abrt; +} + +if (!(local = virStreamNew(data->dconn, VIR_STREAM_NONBLOCK))) +goto abrt; + +if (!(remote = virStreamNew(data->dconn, VIR_STREAM_NONBLOCK))) +goto abrt; + +ret = virDomainMigrateOpenTunnel(data->dconn, + remote, + data->uuid, + VIR_MIGRATE_TUNNEL_NBD); + +if (ret < 0) +goto abrt; + +if (virFDStreamOpen(local, fd) < 0) +goto abrt; + +if (!(pipe = qemuMigrationPipeCreate(local, remote))) +goto abrt; + +pipe->data = data; +pipe->next = data->pipes; +data->pipes = pipe; + +return 0; + + abrt: +VIR_FORCE_CLOSE(fd); +virStreamAbort(local); +virStreamAbort(remote); + +virObjectUnref(local); +virObjectUnref(remote); +return -1; +} + +static void +qemuMigrationPipesStop(qemuMigrationPipePtr pipe, bool abort) +{ +qemuMigrationPipePtr tmp; + +while (pipe) { +tmp = pipe->next; + +qemuMigrationPipeClose(pipe, abort); +VIR_FREE(pipe); + +pipe = tmp; +} +} + +static void qemuMigrationIOFunc(void *arg) { qemuMigrationIOThreadPtr data = arg; @@ -4081,9 +4199,14 @@ qemuMigrationIOFunc(void *arg) break; } } + +if (fds[2].revents & (POLLIN | POLLERR | POLLHUP) && +qemuNBDTunnelAcceptAndPipe(data) < 0) +goto abrt; } virStreamFinish(data->qemuStream); +qemuMigrationPipesStop(data->pipes, false); VIR_FORCE_CLOSE(data->qemuSock); VIR_FREE(buffer); @@ -4097,6 +4220,7 @@ qemuMigrationIOFunc(void *arg) err = NULL; } virStreamAbort(data->qemuStream); +qemuMigrationPipesStop(data->pipes, true); if (err) { virSetError(err); virFreeError(err); @@ -4114,7 +4238,9 @@ qemuMigrationIOFunc(void *arg) static qemuMigrationIOThreadPtr -qemuMigrationStartTunnel(virStreamPtr qemuStream) +qemuMigrationStartTunnel(virStreamPtr qemuStream, + virConnectPtr dconn, + unsigned char uuid[VIR_UUID_BUFLEN]) { qemuMigrationIOThreadPtr io
[libvirt] [PATCH 05/21] domain: add virDomainMigrateOpenTunnel
Add auxiliary private function that calls the apropriate driver's domainMigrateOpenTunnel function. Signed-off-by: Pavel Boldin --- docs/apibuild.py | 1 + docs/hvsupport.pl| 1 + src/libvirt-domain.c | 43 +++ src/libvirt_internal.h | 6 ++ src/libvirt_private.syms | 1 + 5 files changed, 52 insertions(+) diff --git a/docs/apibuild.py b/docs/apibuild.py index f934fb2..6e60093 100755 --- a/docs/apibuild.py +++ b/docs/apibuild.py @@ -102,6 +102,7 @@ ignored_functions = { "virDomainMigratePrepare3Params": "private function for migration", "virDomainMigrateConfirm3Params": "private function for migration", "virDomainMigratePrepareTunnel3Params": "private function for tunnelled migration", + "virDomainMigrateOpenTunnel": "private function for tunnelled migration", "virErrorCopyNew": "private", } diff --git a/docs/hvsupport.pl b/docs/hvsupport.pl index 44a30ce..3b6ee65 100755 --- a/docs/hvsupport.pl +++ b/docs/hvsupport.pl @@ -200,6 +200,7 @@ $apis{virDomainMigratePerform3Params}->{vers} = "1.1.0"; $apis{virDomainMigrateFinish3Params}->{vers} = "1.1.0"; $apis{virDomainMigrateConfirm3Params}->{vers} = "1.1.0"; +$apis{virDomainMigrateOpenTunnel}->{vers} = "1.2.XX"; # Now we want to get the mapping between public APIs diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index de7eb04..3037c01 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -11595,3 +11595,46 @@ virDomainInterfaceFree(virDomainInterfacePtr iface) VIR_FREE(iface); } + + +/* + * Not for public use. This function is part of the internal + * implementation of migration in the remote case. + */ +int +virDomainMigrateOpenTunnel(virConnectPtr conn, + virStreamPtr st, + unsigned char uuid[VIR_UUID_BUFLEN], + unsigned int flags) +{ +char uuidstr[VIR_UUID_STRING_BUFLEN]; + +virUUIDFormat(uuid, uuidstr); +VIR_DEBUG("conn=%p, stream=%p, uuid=%s, flags=%x", + conn, st, uuidstr, flags); + +virResetLastError(); + +virCheckConnectReturn(conn, -1); +virCheckReadOnlyGoto(conn->flags, error); + +if (conn != st->conn) { +virReportInvalidArg(conn, "%s", +_("conn must match stream connection")); +goto error; +} + +if (conn->driver->domainMigrateOpenTunnel) { +int rv; +rv = conn->driver->domainMigrateOpenTunnel(conn, st, uuid, flags); +if (rv < 0) +goto error; +return rv; +} + +virReportUnsupportedError(); + + error: +virDispatchError(conn); +return -1; +} diff --git a/src/libvirt_internal.h b/src/libvirt_internal.h index 1313b58..bbfba0b 100644 --- a/src/libvirt_internal.h +++ b/src/libvirt_internal.h @@ -289,4 +289,10 @@ virTypedParameterValidateSet(virConnectPtr conn, virTypedParameterPtr params, int nparams); +int +virDomainMigrateOpenTunnel(virConnectPtr conn, + virStreamPtr st, + unsigned char uuid[VIR_UUID_BUFLEN], + unsigned int flags); + #endif diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index a835f18..cf5725c 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -952,6 +952,7 @@ virDomainMigrateFinish; virDomainMigrateFinish2; virDomainMigrateFinish3; virDomainMigrateFinish3Params; +virDomainMigrateOpenTunnel; virDomainMigratePerform; virDomainMigratePerform3; virDomainMigratePerform3Params; -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 12/21] qemu: migration: src: add NBD unixSock to iothread
Pass UNIX socket used as a local NBD server destination to the migration iothread. Signed-off-by: Pavel Boldin --- src/qemu/qemu_migration.c | 46 ++ 1 file changed, 38 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 61e78c5..0682fd8 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3993,6 +3993,7 @@ struct _qemuMigrationIOThread { virThread thread; virStreamPtr qemuStream; int qemuSock; +int unixSock; virError err; int wakeupRecvFD; int wakeupSendFD; @@ -4003,7 +4004,7 @@ qemuMigrationIOFunc(void *arg) { qemuMigrationIOThreadPtr data = arg; char *buffer = NULL; -struct pollfd fds[2]; +struct pollfd fds[3]; int timeout = -1; virErrorPtr err = NULL; @@ -4013,8 +4014,8 @@ qemuMigrationIOFunc(void *arg) goto abrt; fds[0].fd = data->wakeupRecvFD; -fds[1].fd = -1; -fds[0].events = fds[1].events = POLLIN; +fds[1].fd = fds[2].fd = -1; +fds[0].events = fds[1].events = fds[2].events = POLLIN; for (;;) { int ret; @@ -4057,7 +4058,9 @@ qemuMigrationIOFunc(void *arg) break; case 'u': fds[1].fd = data->qemuSock; -VIR_DEBUG("qemuSock set %d", data->qemuSock); +fds[2].fd = data->unixSock; +VIR_DEBUG("qemuSock set %d, unixSock set %d", + data->qemuSock, data->unixSock); break; } } @@ -4126,7 +4129,7 @@ qemuMigrationStartTunnel(virStreamPtr qemuStream) goto error; io->qemuStream = qemuStream; -io->qemuSock = -1; +io->qemuSock = io->unixSock = -1; io->wakeupRecvFD = wakeupFD[0]; io->wakeupSendFD = wakeupFD[1]; @@ -4202,6 +4205,26 @@ qemuMigrationSetQEMUSocket(qemuMigrationIOThreadPtr io, int sock) } static int +qemuMigrationSetUnixSocket(qemuMigrationIOThreadPtr io, int sock) +{ +int rv = -1; +char action = 'u'; + +io->unixSock = sock; + +if (safewrite(io->wakeupSendFD, &action, 1) != 1) { +virReportSystemError(errno, "%s", + _("failed to update migration tunnel")); +goto error; +} + +rv = 0; + + error: +return rv; +} + +static int qemuMigrationConnect(virQEMUDriverPtr driver, virDomainObjPtr vm, qemuMigrationSpecPtr spec) @@ -4313,6 +4336,16 @@ qemuMigrationRun(virQEMUDriverPtr driver, if (qemuDomainMigrateGraphicsRelocate(driver, vm, mig, graphicsuri) < 0) VIR_WARN("unable to provide data for graphics client relocation"); +if (spec->fwdType != MIGRATION_FWD_DIRECT) { +if (!(iothread = qemuMigrationStartTunnel(spec->fwd.stream))) +goto cancel; + +if (nmigrate_disks && +qemuMigrationSetUnixSocket(iothread, + spec->nbd_tunnel_unix_socket.sock) < 0) +goto cancel; +} + if (migrate_flags & (QEMU_MONITOR_MIGRATE_NON_SHARED_DISK | QEMU_MONITOR_MIGRATE_NON_SHARED_INC)) { bool dest_host = spec->destType == MIGRATION_DEST_HOST; @@ -,9 +4477,6 @@ qemuMigrationRun(virQEMUDriverPtr driver, } if (spec->fwdType != MIGRATION_FWD_DIRECT) { -if (!(iothread = qemuMigrationStartTunnel(spec->fwd.stream))) -goto cancel; - if (qemuMigrationSetQEMUSocket(iothread, fd) < 0) goto cancel; /* If we've created a tunnel, then the 'fd' will be closed in the -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 06/21] domain: add virDomainMigrateTunnelFlags
Add virDomainMigrateTunnelFlags enum. Signed-off-by: Pavel Boldin --- include/libvirt/libvirt-domain.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index a1ea6a5..444deee 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -661,6 +661,9 @@ typedef enum { VIR_MIGRATE_RDMA_PIN_ALL = (1 << 14), /* RDMA memory pinning */ } virDomainMigrateFlags; +typedef enum { +VIR_MIGRATE_TUNNEL_NBD= (1 << 0), /* open tunnel for NBD */ +} virDomainMigrateTunnelFlags; /** * VIR_MIGRATE_PARAM_URI: -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 10/21] qemu: migration: src: qemu `drive-mirror` to UNIX
Make qemuMigrationDriveMirror able to instruct QEMU to connect to a local UNIX socket used for tunnelling. Signed-off-by: Pavel Boldin --- src/qemu/qemu_migration.c | 45 ++--- 1 file changed, 30 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index d587c56..d95cd66 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2048,7 +2048,8 @@ static int qemuMigrationDriveMirror(virQEMUDriverPtr driver, virDomainObjPtr vm, qemuMigrationCookiePtr mig, - const char *host, + bool dest_host, + const char *dest, unsigned long speed, unsigned int *migrate_flags, size_t nmigrate_disks, @@ -2057,7 +2058,7 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver, { qemuDomainObjPrivatePtr priv = vm->privateData; int ret = -1; -int port; +int port = -1; size_t i; char *diskAlias = NULL; char *nbd_dest = NULL; @@ -2068,16 +2069,20 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver, VIR_DEBUG("Starting drive mirrors for domain %s", vm->def->name); -/* steal NBD port and thus prevent its propagation back to destination */ -port = mig->nbd->port; -mig->nbd->port = 0; +virCheckNonNullArgGoto(dest, cleanup); + +if (dest_host) { +/* steal NBD port and thus prevent its propagation back to destination */ +port = mig->nbd->port; +mig->nbd->port = 0; -/* escape literal IPv6 address */ -if (strchr(host, ':')) { -if (virAsprintf(&hoststr, "[%s]", host) < 0) +/* escape literal IPv6 address */ +if (strchr(dest, ':')) { +if (virAsprintf(&hoststr, "[%s]", dest) < 0) +goto cleanup; +} else if (VIR_STRDUP(hoststr, dest) < 0) { goto cleanup; -} else if (VIR_STRDUP(hoststr, host) < 0) { -goto cleanup; +} } if (*migrate_flags & QEMU_MONITOR_MIGRATE_NON_SHARED_INC) @@ -2092,11 +2097,18 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver, if (!qemuMigrateDisk(disk, nmigrate_disks, migrate_disks)) continue; -if ((virAsprintf(&diskAlias, "%s%s", - QEMU_DRIVE_HOST_PREFIX, disk->info.alias) < 0) || -(virAsprintf(&nbd_dest, "nbd:%s:%d:exportname=%s", - hoststr, port, diskAlias) < 0)) +if (virAsprintf(&diskAlias, "%s%s", +QEMU_DRIVE_HOST_PREFIX, disk->info.alias) < 0) goto cleanup; +if (dest_host) { +if (virAsprintf(&nbd_dest, "nbd:%s:%d:exportname=%s", +hoststr, port, diskAlias) < 0) +goto cleanup; +} else { +if (virAsprintf(&nbd_dest, "nbd:unix:%s:exportname=%s", +dest, diskAlias) < 0) +goto cleanup; +} if (qemuDomainObjEnterMonitorAsync(driver, vm, QEMU_ASYNC_JOB_MIGRATION_OUT) < 0) @@ -4281,10 +4293,13 @@ qemuMigrationRun(virQEMUDriverPtr driver, if (migrate_flags & (QEMU_MONITOR_MIGRATE_NON_SHARED_DISK | QEMU_MONITOR_MIGRATE_NON_SHARED_INC)) { +bool dest_host = spec->destType == MIGRATION_DEST_HOST; +const char *dest = dest_host ? spec->dest.host.name : + spec->nbd_tunnel_unix_socket.file; if (mig->nbd) { /* This will update migrate_flags on success */ if (qemuMigrationDriveMirror(driver, vm, mig, - spec->dest.host.name, + dest_host, dest, migrate_speed, &migrate_flags, nmigrate_disks, -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 08/21] qemu: migration: src: add nbd tunnel socket data
Add local NBD tunnel socket info to the qemuMigrationSpec structure. Signed-off-by: Pavel Boldin --- src/qemu/qemu_migration.c | 5 + 1 file changed, 5 insertions(+) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 3eee3a5..fb2a216 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3966,6 +3966,11 @@ struct _qemuMigrationSpec { union { virStreamPtr stream; } fwd; + +struct { +char *file; +int sock; +} nbd_tunnel_unix_socket; }; #define TUNNEL_SEND_BUF_SIZE 65536 -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 03/21] remote_driver: introduce virRemoteClientNew
Move common code to a function. Signed-off-by: Pavel Boldin --- src/remote/remote_driver.c | 48 +++--- 1 file changed, 28 insertions(+), 20 deletions(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index a1dd640..b72cb86 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -6119,6 +6119,28 @@ remoteDomainMigratePrepare3(virConnectPtr dconn, goto done; } +static virNetClientStreamPtr +virRemoteClientOpen(virStreamPtr st, +struct private_data *priv, +enum remote_procedure proc) +{ +virNetClientStreamPtr netst; + +if (!(netst = virNetClientStreamNew(priv->remoteProgram, +proc, +priv->counter))) +return NULL; + +if (virNetClientAddStream(priv->client, netst) < 0) { +virObjectUnref(netst); +return NULL; +} + +st->driver = &remoteStreamDrv; +st->privateData = netst; + +return netst; +} static int remoteDomainMigratePrepareTunnel3(virConnectPtr dconn, @@ -6143,18 +6165,11 @@ remoteDomainMigratePrepareTunnel3(virConnectPtr dconn, memset(&args, 0, sizeof(args)); memset(&ret, 0, sizeof(ret)); -if (!(netst = virNetClientStreamNew(priv->remoteProgram, - REMOTE_PROC_DOMAIN_MIGRATE_PREPARE_TUNNEL3, -priv->counter))) -goto done; +netst = virRemoteClientOpen(st, priv, +REMOTE_PROC_DOMAIN_MIGRATE_PREPARE_TUNNEL3); -if (virNetClientAddStream(priv->client, netst) < 0) { -virObjectUnref(netst); +if (netst == NULL) goto done; -} - -st->driver = &remoteStreamDrv; -st->privateData = netst; args.cookie_in.cookie_in_val = (char *)cookiein; args.cookie_in.cookie_in_len = cookieinlen; @@ -7193,18 +7208,11 @@ remoteDomainMigratePrepareTunnel3Params(virConnectPtr dconn, goto cleanup; } -if (!(netst = virNetClientStreamNew(priv->remoteProgram, - REMOTE_PROC_DOMAIN_MIGRATE_PREPARE_TUNNEL3_PARAMS, -priv->counter))) -goto cleanup; +netst = virRemoteClientOpen(st, priv, + REMOTE_PROC_DOMAIN_MIGRATE_PREPARE_TUNNEL3_PARAMS); -if (virNetClientAddStream(priv->client, netst) < 0) { -virObjectUnref(netst); +if (netst == NULL) goto cleanup; -} - -st->driver = &remoteStreamDrv; -st->privateData = netst; if (call(dconn, priv, 0, REMOTE_PROC_DOMAIN_MIGRATE_PREPARE_TUNNEL3_PARAMS, (xdrproc_t) xdr_remote_domain_migrate_prepare_tunnel3_params_args, -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC] Design: Tunnelled NBD block disk migration
Michal, Please see my comments intermixed below. > > > 1. `Pipe`s used to interconnect to the QEMU on the both sides are obviously > > to be replaced by the UNIX sockets since the pipes cannot support > > bidirectional output due to the design. This is to be made *one for each* > > block device, requiring the above change. > > I'm not sure. Does qemu require bidirectional stream? I guess it should > so that the source is notified about disks switching to mirroring phase. > If that's the case, then you're right. As far as I see the QEMU migration process only uses one-way communication. But I have a feeling that the NBD actually transfers data both ways. > > > > 2. The `qemuMigrationIOFunc` must be rewritten in such a way it polls for > > both read and write on the UNIX sockets *and* VM migration pipe and > > tunnells these to the appropriate `virStream`. > > 3. The receiving code must be modified in such a way to tunnel each of the > > opened UNIX socket to the according virFDStream. > > > > Of the mentioned above the most intriguing for me is the zero-th item which > > will require some changes to the binary remote messaging protocol. > > > Yeah, this is something we really try to avoid. The reason is that older > clients remain compatible with newer servers and vice versa. If we > invent new streams, we should probably invent new RPC calls too; > virBiStream or something like this. This way we can still serve data to > older clients (e.g. screenshots are transferred through virStream and > mangling it will cut off older clients) and use the new feature - in > which case older clients will fail with unknown rpc call. Well, this seems to be the way to do it: implement an RPC call that converts the remoteStream into multiplexing one. This call is to be performed by the migration source when there is the appropriate capability set on the remote's side. Does this looks correct for you? Pavel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [RFC] Design: Tunnelled NBD block disk migration
Dear All, I continue my work on the tunnelled selective block disks migration and the next step is to implement Tunnelled NBD block disk migration. As far as I see libvirt uses the following algorithm for tunnelling the QEMU migration stream that is unidirectional: 1. The qemuMigrationStartTunnel (src/qemu/qemu_migration.c) starts a thread that reads the data from the local QEMU pipe and writes it to the virStream using virStreamSend. 2. The stream used by virStreamSend is embedded in the remote protocol messaging by the remoteDomainMigratePrepareTunnel3{,Params} (src/remote/remote_driver.c). 3. Remote links the daemonStream with the virFDStream connected to the write end of a pipe linked with the QEMU. 4. Overall stream path is then: QEMU -(pipe)> virStreamSend -> (message passing to the remote) -> TCP -> (message received by deamon/*.c code) -> virStreamSend -> virFDStreamWrite -(pipe)> QEMU. Given that I wonder what should be the changes required to implement a bidirectional QEMU tunnelled connection for the NBD device disk migration. The way I see implementation details at the moment: 0. The `virStream` and corresponding `daemon/*.c` must be modified in a way that multiple streams can be multiplexed through the only one remote connection. 1. `Pipe`s used to interconnect to the QEMU on the both sides are obviously to be replaced by the UNIX sockets since the pipes cannot support bidirectional output due to the design. This is to be made *one for each* block device, requiring the above change. 2. The `qemuMigrationIOFunc` must be rewritten in such a way it polls for both read and write on the UNIX sockets *and* VM migration pipe and tunnells these to the appropriate `virStream`. 3. The receiving code must be modified in such a way to tunnel each of the opened UNIX socket to the according virFDStream. Of the mentioned above the most intriguing for me is the zero-th item which will require some changes to the binary remote messaging protocol. Is my vision on the problem is correct? Are there any other difficulties I'm going to face but not aware of due to the lack of the familiarity with the code? Pavel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH libvirt-python] virPyDictToTypedParams: packing lists of values
Pack a list or a tuple of values passed to a Python method to the multi-value parameter. --- libvirt-override.c | 228 ++--- 1 file changed, 129 insertions(+), 99 deletions(-) diff --git a/libvirt-override.c b/libvirt-override.c index 588dac1..45c8afc 100644 --- a/libvirt-override.c +++ b/libvirt-override.c @@ -278,6 +278,126 @@ typedef virPyTypedParamsHint *virPyTypedParamsHintPtr; # define libvirt_PyString_Check PyString_Check # endif +static int +virPyDictToTypedParamOne(virTypedParameterPtr *params, + int *n, + int *max, + virPyTypedParamsHintPtr hints, + int nhints, + const char *keystr, + PyObject *value) +{ +int rv = -1, type = -1; +size_t i; + +for (i = 0; i < nhints; i++) { +if (STREQ(hints[i].name, keystr)) { +type = hints[i].type; +break; +} +} + +if (type == -1) { +if (libvirt_PyString_Check(value)) { +type = VIR_TYPED_PARAM_STRING; +} else if (PyBool_Check(value)) { +type = VIR_TYPED_PARAM_BOOLEAN; +} else if (PyLong_Check(value)) { +unsigned long long ull = PyLong_AsUnsignedLongLong(value); +if (ull == (unsigned long long) -1 && PyErr_Occurred()) +type = VIR_TYPED_PARAM_LLONG; +else +type = VIR_TYPED_PARAM_ULLONG; +#if PY_MAJOR_VERSION < 3 +} else if (PyInt_Check(value)) { +if (PyInt_AS_LONG(value) < 0) +type = VIR_TYPED_PARAM_LLONG; +else +type = VIR_TYPED_PARAM_ULLONG; +#endif +} else if (PyFloat_Check(value)) { +type = VIR_TYPED_PARAM_DOUBLE; +} +} + +if (type == -1) { +PyErr_Format(PyExc_TypeError, + "Unknown type of \"%s\" field", keystr); +goto cleanup; +} + +switch ((virTypedParameterType) type) { +case VIR_TYPED_PARAM_INT: +{ +int val; +if (libvirt_intUnwrap(value, &val) < 0 || +virTypedParamsAddInt(params, n, max, keystr, val) < 0) +goto cleanup; +break; +} +case VIR_TYPED_PARAM_UINT: +{ +unsigned int val; +if (libvirt_uintUnwrap(value, &val) < 0 || +virTypedParamsAddUInt(params, n, max, keystr, val) < 0) +goto cleanup; +break; +} +case VIR_TYPED_PARAM_LLONG: +{ +long long val; +if (libvirt_longlongUnwrap(value, &val) < 0 || +virTypedParamsAddLLong(params, n, max, keystr, val) < 0) +goto cleanup; +break; +} +case VIR_TYPED_PARAM_ULLONG: +{ +unsigned long long val; +if (libvirt_ulonglongUnwrap(value, &val) < 0 || +virTypedParamsAddULLong(params, n, max, keystr, val) < 0) +goto cleanup; +break; +} +case VIR_TYPED_PARAM_DOUBLE: +{ +double val; +if (libvirt_doubleUnwrap(value, &val) < 0 || +virTypedParamsAddDouble(params, n, max, keystr, val) < 0) +goto cleanup; +break; +} +case VIR_TYPED_PARAM_BOOLEAN: +{ +bool val; +if (libvirt_boolUnwrap(value, &val) < 0 || +virTypedParamsAddBoolean(params, n, max, keystr, val) < 0) +goto cleanup; +break; +} +case VIR_TYPED_PARAM_STRING: +{ +char *val;; +if (libvirt_charPtrUnwrap(value, &val) < 0 || +!val || +virTypedParamsAddString(params, n, max, keystr, val) < 0) { +VIR_FREE(val); +goto cleanup; +} +VIR_FREE(val); +break; +} +case VIR_TYPED_PARAM_LAST: +break; /* unreachable */ +} + +rv = 0; + + cleanup: +return rv; +} + + /* Automatically convert dict into type parameters based on types reported * by python. All integer types are converted into LLONG (in case of a negative * value) or ULLONG (in case of a positive value). If you need different @@ -300,7 +420,6 @@ virPyDictToTypedParams(PyObject *dict, Py_ssize_t pos = 0; #endif virTypedParameterPtr params = NULL; -size_t i; int n = 0; int max = 0; int ret = -1; @@ -313,112 +432,23 @@ virPyDictToTypedParams(PyObject *dict, return -1; while (PyDict_Next(dict, &pos, &key, &value)) { -int type = -1; - if (libvirt_charPtrUnwrap(key, &keystr) < 0 || !keystr) goto cleanup; -for (i = 0; i < nhints; i++) { -if (STREQ(hints[i].name, keystr)) { -type = hints[i].type; -break; -} -} +if (PyList_Check(value) || PyTuple_Check(value)) { +Py_ssize_t i, size = PySequence_Size(value); -if (type == -1) { -if (libvirt_PyString_C
[libvirt] [PATCH v4 9/9] virsh: selective block device migration
Add `virsh migrate' option `--migratedisks' that allows CLI user to explicitly specify block devices to migrate. Signed-off-by: Pavel Boldin Signed-off-by: Michal Privoznik --- tools/virsh-domain.c | 23 +++ tools/virsh.pod | 21 - 2 files changed, 35 insertions(+), 9 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index a25b7ba..2dd2eea 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -9806,6 +9806,10 @@ static const vshCmdOptDef opts_migrate[] = { .type = VSH_OT_STRING, .help = N_("filename containing updated XML for the target") }, +{.name = "migratedisks", + .type = VSH_OT_STRING, + .help = N_("comma separated list of disks to be migrated") +}, {.name = NULL} }; @@ -9865,6 +9869,25 @@ doMigrate(void *opaque) VIR_MIGRATE_PARAM_DEST_NAME, opt) < 0) goto save_error; +if (vshCommandOptStringReq(ctl, cmd, "migratedisks", &opt) < 0) +goto out; +if (opt) { +char **val = NULL; + +val = virStringSplit(opt, ",", 0); + +if (virTypedParamsAddStringList(¶ms, +&nparams, +&maxparams, +VIR_MIGRATE_PARAM_MIGRATE_DISKS, +(const char **)val) < 0) { +VIR_FREE(val); +goto save_error; +} + +VIR_FREE(val); +} + if (vshCommandOptStringReq(ctl, cmd, "xml", &opt) < 0) goto out; if (opt) { diff --git a/tools/virsh.pod b/tools/virsh.pod index 4e3f82a..6f87c09 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1521,6 +1521,7 @@ to the I namespace is displayed instead of being modified. [I<--compressed>] [I<--abort-on-error>] [I<--auto-converge>] I I [I] [I] [I] [I] [I<--timeout> B] [I<--xml> B] +[I<--migratedisks> B] Migrate domain to another host. Add I<--live> for live migration; <--p2p> for peer-2-peer migration; I<--direct> for direct migration; or I<--tunnelled> @@ -1536,15 +1537,17 @@ with incremental copy (same base image shared between source and destination). In both cases the disk images have to exist on destination host, the I<--copy-storage-...> options only tell libvirt to transfer data from the images on source host to the images found at the same place on the destination -host. I<--change-protection> enforces that no incompatible configuration -changes will be made to the domain while the migration is underway; this flag -is implicitly enabled when supported by the hypervisor, but can be explicitly -used to reject the migration if the hypervisor lacks change protection -support. I<--verbose> displays the progress of migration. I<--compressed> -activates compression of memory pages that have to be transferred repeatedly -during live migration. I<--abort-on-error> cancels the migration if a soft -error (for example I/O error) happens during the migration. I<--auto-converge> -forces convergence during live migration. +host. By default only non-shared non-readonly images are transferred. Use +I<--migratedisks> to explicitly specify a list of disk targets to +transfer via the comma separated B argument. I<--change-protection> +enforces that no incompatible configuration changes will be made to the domain +while the migration is underway; this flag is implicitly enabled when supported +by the hypervisor, but can be explicitly used to reject the migration if the +hypervisor lacks change protection support. I<--verbose> displays the progress +of migration. I<--compressed> activates compression of memory pages that have +to be transferred repeatedly during live migration. I<--abort-on-error> cancels +the migration if a soft error (for example I/O error) happens during the +migration. I<--auto-converge> forces convergence during live migration. B: Individual hypervisors usually do not support all possible types of migration. For example, QEMU does not support direct migration. -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4 3/9] qemuMigrationDriveMirror: Force raw format for NBD
From: Michal Privoznik When playing with disk migration lately, I've noticed this warning in domain logs: WARNING: Image format was not specified for 'nbd://masina:49153/drive-virtio-disk0' and probing guessed raw. Automatically detecting the format is dangerous for raw images, write operations on block 0 will be restricted. Specify the 'raw' format explicitly to remove the restrictions. So I started digging into qemu source code to see what has triggered the warning. I'd expect qemu to know formats of guest's disks since we tell them on command line. This lead me to qmp_drive_mirror() where the following can be found: if (!has_format) { format = mode == NEW_IMAGE_MODE_EXISTING ? NULL : bs->drv->format_name; } So, format is automatically initialized from the disk iff mode != "existing". Unfortunately, in migration we are tied to use this mode (NBD doesn't support creating new images). Therefore the only way to avoid this warning is to pass format. The discussion on the mail-list [1] resulted in the code that always forces NBD export as "raw" format. [1] https://www.redhat.com/archives/libvir-list/2015-June/msg00153.html Signed-off-by: Michal Privoznik Signed-off-by: Pavel Boldin --- src/qemu/qemu_migration.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 6c94052..1fa5e5f 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1966,8 +1966,9 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver, goto cleanup; } +/* Force "raw" format for NBD export */ mon_ret = qemuMonitorDriveMirror(priv->mon, diskAlias, nbd_dest, - NULL, speed, 0, 0, mirror_flags); + "raw", speed, 0, 0, mirror_flags); if (qemuDomainObjExitMonitor(driver, vm) < 0 || mon_ret < 0) { qemuBlockJobSyncEnd(driver, vm, disk, NULL); -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4 2/9] qemuMigrationBeginPhase: Fix function header indentation
From: Michal Privoznik This function is returning a string (domain XML). Since d3ce7363 when it was first introduced, it was indented incorrectly: static char *qemuMigrationBeginPhase(..) Signed-off-by: Michal Privoznik --- src/qemu/qemu_migration.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 70400f3..6c94052 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2699,14 +2699,14 @@ qemuMigrationCleanup(virDomainObjPtr vm, /* The caller is supposed to lock the vm and start a migration job. */ -static char -*qemuMigrationBeginPhase(virQEMUDriverPtr driver, - virDomainObjPtr vm, - const char *xmlin, - const char *dname, - char **cookieout, - int *cookieoutlen, - unsigned long flags) +static char * +qemuMigrationBeginPhase(virQEMUDriverPtr driver, +virDomainObjPtr vm, +const char *xmlin, +const char *dname, +char **cookieout, +int *cookieoutlen, +unsigned long flags) { char *rv = NULL; qemuMigrationCookiePtr mig = NULL; -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4 6/9] util: virTypedParams{Filter, GetAllStrings}
Add multikey API: * virTypedParamsFilter that filters all the parameters with specified name. * virTypedParamsGetAllStrings that returns a list with all the values for specified name and string type. Signed-off-by: Pavel Boldin Signed-off-by: Michal Privoznik --- include/libvirt/libvirt-host.h | 5 ++ src/libvirt_public.syms| 5 ++ src/util/virtypedparam.c | 102 + src/util/virtypedparam.h | 9 tests/Makefile.am | 2 +- tests/virtypedparamtest.c | 100 6 files changed, 222 insertions(+), 1 deletion(-) diff --git a/include/libvirt/libvirt-host.h b/include/libvirt/libvirt-host.h index 070550b..8222cfb 100644 --- a/include/libvirt/libvirt-host.h +++ b/include/libvirt/libvirt-host.h @@ -284,6 +284,11 @@ virTypedParamsGetString (virTypedParameterPtr params, const char *name, const char **value); int +virTypedParamsGetAllStrings(virTypedParameterPtr params, + int nparams, + const char *name, + const char ***values); +int virTypedParamsAddInt(virTypedParameterPtr *params, int *nparams, int *maxparams, diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 716dd2f..0a1feea 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -715,4 +715,9 @@ LIBVIRT_1.2.16 { virDomainSetUserPassword; } LIBVIRT_1.2.15; +LIBVIRT_1.3.0 { +global: +virTypedParamsGetAllStrings; +} LIBVIRT_1.2.16; + # define new API here using predicted next version number diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c index 6f608d6..a12006c 100644 --- a/src/util/virtypedparam.c +++ b/src/util/virtypedparam.c @@ -482,6 +482,51 @@ virTypedParamsGet(virTypedParameterPtr params, } +/** + * virTypedParamsFilter: + * @params: array of typed parameters + * @nparams: number of parameters in the @params array + * @name: name of the parameter to find + * @ret: pointer to the returned array + * + * Filters @params retaining only the parameters named @name in the + * resulting array @ret. Caller should free the @ret array but not + * the items since they are pointing to the @params elements. + * + * Returns amount of elements in @ret on success, -1 on error. + */ +int +virTypedParamsFilter(virTypedParameterPtr params, + int nparams, + const char *name, + virTypedParameterPtr **ret) +{ +size_t i, alloc = 0, n = 0; + +virCheckNonNullArgGoto(params, error); +virCheckNonNullArgGoto(name, error); +virCheckNonNullArgGoto(ret, error); + +*ret = NULL; + +for (i = 0; i < nparams; i++) { +if (STREQ(params[i].field, name)) { +if (VIR_RESIZE_N(*ret, alloc, n, 1) < 0) +goto error; + +(*ret)[n] = ¶ms[i]; + +n++; +} +} + +return n; + + error: +return -1; +} + + #define VIR_TYPED_PARAM_CHECK_TYPE(check_type) \ do { if (param->type != check_type) { \ virReportError(VIR_ERR_INVALID_ARG, \ @@ -750,6 +795,63 @@ virTypedParamsGetString(virTypedParameterPtr params, /** + * virTypedParamsGetAllStrings: + * @params: array of typed parameters + * @nparams: number of parameters in the @params array + * @name: name of the parameter to find + * @values: array of returned values + * + * Finds all parameters with desired @name within @params and + * store their values into @values. The @values array is self + * allocated and its length is stored into @picked. When no + * longer needed, caller should free the returned array, but not + * the items since they are taken from @params array. + * + * Returns amount of strings in @values array on success, + * -1 otherwise. + */ +int +virTypedParamsGetAllStrings(virTypedParameterPtr params, +int nparams, +const char *name, +const char ***values) +{ +size_t i, n; +int nfiltered; +virTypedParameterPtr *filtered = NULL; + +virResetLastError(); + +virCheckNonNullArgGoto(values, error); +*values = NULL; + +nfiltered = virTypedParamsFilter(params, nparams, name, &filtered); + +if (nfiltered < 0) +goto error; + +if (nfiltered && +VIR_ALLOC_N(*values, nfiltered) < 0) +goto error; + +for (n = 0, i = 0; i < nfiltered; i++) { +if (filtered[i]->type == VIR_TYPED_PARAM_STRING) +(*values)[n++] = filtered[i]->value.s; +} + +VIR_FREE(filtered); +return n; + + error: +if (values) +VIR_FREE(*values); +VIR_FREE(filtered); +vi
[libvirt] [PATCH v4 4/9] util: multi-value virTypedParameter
The `virTypedParamsValidate' function now can be instructed to allow multiple entries for some of the keys. For this flag the type with the `VIR_TYPED_PARAM_MULTIPLE' flag. Add unit tests for this new behaviour. Signed-off-by: Pavel Boldin Signed-off-by: Michal Privoznik --- src/util/virtypedparam.c | 109 +++--- src/util/virtypedparam.h | 10 +++ tests/Makefile.am | 6 ++ tests/virtypedparamtest.c | 167 ++ 4 files changed, 253 insertions(+), 39 deletions(-) create mode 100644 tests/virtypedparamtest.c diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c index de2d447..68620f5 100644 --- a/src/util/virtypedparam.c +++ b/src/util/virtypedparam.c @@ -47,11 +47,19 @@ VIR_ENUM_IMPL(virTypedParameter, VIR_TYPED_PARAM_LAST, * internal utility functions (those in libvirt_private.syms) may * report errors that the caller will dispatch. */ +static int +virTypedParamsSortName(const void *left, const void *right) +{ +const virTypedParameter *param_left = left, *param_right = right; +return strcmp(param_left->field, param_right->field); +} + /* Validate that PARAMS contains only recognized parameter names with - * correct types, and with no duplicates. Pass in as many name/type - * pairs as appropriate, and pass NULL to end the list of accepted - * parameters. Return 0 on success, -1 on failure with error message - * already issued. */ + * correct types, and with no duplicates except for parameters + * specified with VIR_TYPED_PARAM_MULTIPLE flag in type. + * Pass in as many name/type pairs as appropriate, and pass NULL to end + * the list of accepted parameters. Return 0 on success, -1 on failure + * with error message already issued. */ int virTypedParamsValidate(virTypedParameterPtr params, int nparams, ...) { @@ -60,60 +68,83 @@ virTypedParamsValidate(virTypedParameterPtr params, int nparams, ...) size_t i, j; const char *name; int type; +size_t nkeys = 0, nkeysalloc = 0; +virTypedParameterPtr sorted = NULL, keys = NULL; va_start(ap, nparams); -/* Yes, this is quadratic, but since we reject duplicates and - * unknowns, it is constrained by the number of var-args passed - * in, which is expected to be small enough to not be - * noticeable. */ -for (i = 0; i < nparams; i++) { -va_end(ap); -va_start(ap, nparams); +if (VIR_ALLOC_N(sorted, nparams) < 0) +goto cleanup; -name = va_arg(ap, const char *); -while (name) { -type = va_arg(ap, int); -if (STREQ(params[i].field, name)) { -if (params[i].type != type) { -const char *badtype; - -badtype = virTypedParameterTypeToString(params[i].type); -if (!badtype) -badtype = virTypedParameterTypeToString(0); -virReportError(VIR_ERR_INVALID_ARG, - _("invalid type '%s' for parameter '%s', " - "expected '%s'"), - badtype, params[i].field, - virTypedParameterTypeToString(type)); -} -break; -} -name = va_arg(ap, const char *); -} -if (!name) { -virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, - _("parameter '%s' not supported"), - params[i].field); +/* Here we intentionally don't copy values */ +memcpy(sorted, params, sizeof(*params) * nparams); +qsort(sorted, nparams, sizeof(*sorted), virTypedParamsSortName); + +name = va_arg(ap, const char *); +while (name) { +type = va_arg(ap, int); +if (VIR_RESIZE_N(keys, nkeysalloc, nkeys, 1) < 0) +goto cleanup; + +if (virStrcpyStatic(keys[nkeys].field, name) == NULL) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _("Field name '%s' too long"), name); goto cleanup; } -for (j = 0; j < i; j++) { -if (STREQ(params[i].field, params[j].field)) { + +keys[nkeys].type = type & ~VIR_TYPED_PARAM_MULTIPLE; +/* Value is not used anyway */ +keys[nkeys].value.i = type & VIR_TYPED_PARAM_MULTIPLE; + +nkeys++; +name = va_arg(ap, const char *); +} + +qsort(keys, nkeys, sizeof(*keys), virTypedParamsSortName); + +for (i = 0, j = 0; i < nparams && j < nkeys;) { +if (STRNEQ(sorted[i].field, keys[j].field)) { +j++; +} else { +if (i > j && !(keys[j].value.i & VIR_TYPED_PARAM_MULTIPLE)) { virReportError(VIR_ERR_INVALID_ARG,
[libvirt] [PATCH v4 8/9] qemu: migration: selective block device migration
Implement a `migrate_disks' parameters for the QEMU driver. This multi- value parameter can be used to explicitly specify what block devices are to be migrated using the NBD server. Tunnelled migration using NBD is to be done. Signed-off-by: Pavel Boldin Signed-off-by: Michal Privoznik --- include/libvirt/libvirt-domain.h | 9 ++ src/qemu/qemu_driver.c | 78 ++--- src/qemu/qemu_migration.c| 245 --- src/qemu/qemu_migration.h| 24 ++-- 4 files changed, 264 insertions(+), 92 deletions(-) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index d851225..7564c20 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -748,6 +748,15 @@ typedef enum { */ # define VIR_MIGRATE_PARAM_LISTEN_ADDRESS"listen_address" +/** + * VIR_MIGRATE_PARAM_MIGRATE_DISKS: + * + * virDomainMigrate* params multiple field: The multiple values that list + * the block devices to be migrated. At the moment this is only supported + * by the QEMU driver but not for the tunnelled migration. + */ +# define VIR_MIGRATE_PARAM_MIGRATE_DISKS"migrate_disks" + /* Domain migration. */ virDomainPtr virDomainMigrate (virDomainPtr domain, virConnectPtr dconn, unsigned long flags, const char *dname, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 34e5581..c244f46 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12226,7 +12226,7 @@ qemuDomainMigratePrepare2(virConnectPtr dconn, ret = qemuMigrationPrepareDirect(driver, dconn, NULL, 0, NULL, NULL, /* No cookies */ uri_in, uri_out, - &def, origname, NULL, flags); + &def, origname, NULL, 0, NULL, flags); cleanup: VIR_FREE(origname); @@ -12279,7 +12279,7 @@ qemuDomainMigratePerform(virDomainPtr dom, * Consume any cookie we were able to decode though */ ret = qemuMigrationPerform(driver, dom->conn, vm, - NULL, dconnuri, uri, NULL, NULL, + NULL, dconnuri, uri, NULL, NULL, 0, NULL, cookie, cookielen, NULL, NULL, /* No output cookies in v2 */ flags, dname, resource, false); @@ -12356,7 +12356,7 @@ qemuDomainMigrateBegin3(virDomainPtr domain, } return qemuMigrationBegin(domain->conn, vm, xmlin, dname, - cookieout, cookieoutlen, flags); + cookieout, cookieoutlen, flags, 0, NULL); } static char * @@ -12369,11 +12369,14 @@ qemuDomainMigrateBegin3Params(virDomainPtr domain, { const char *xmlin = NULL; const char *dname = NULL; +const char **migrate_disks = NULL; +int nmigrate_disks; +char *ret = NULL; virDomainObjPtr vm; virCheckFlags(QEMU_MIGRATION_FLAGS, NULL); if (virTypedParamsValidate(params, nparams, QEMU_MIGRATION_PARAMETERS) < 0) -return NULL; +goto cleanup; if (virTypedParamsGetString(params, nparams, VIR_MIGRATE_PARAM_DEST_XML, @@ -12381,18 +12384,30 @@ qemuDomainMigrateBegin3Params(virDomainPtr domain, virTypedParamsGetString(params, nparams, VIR_MIGRATE_PARAM_DEST_NAME, &dname) < 0) -return NULL; +goto cleanup; + +nmigrate_disks = virTypedParamsGetAllStrings( +params, nparams, VIR_MIGRATE_PARAM_MIGRATE_DISKS, +&migrate_disks); + +if (nmigrate_disks < 0) +goto cleanup; if (!(vm = qemuDomObjFromDomain(domain))) -return NULL; +goto cleanup; if (virDomainMigrateBegin3ParamsEnsureACL(domain->conn, vm->def) < 0) { virDomainObjEndAPI(&vm); -return NULL; +goto cleanup; } -return qemuMigrationBegin(domain->conn, vm, xmlin, dname, - cookieout, cookieoutlen, flags); +ret = qemuMigrationBegin(domain->conn, vm, xmlin, dname, + cookieout, cookieoutlen, flags, + nmigrate_disks, migrate_disks); + + cleanup: +VIR_FREE(migrate_disks); +return ret; } @@ -12436,7 +12451,7 @@ qemuDomainMigratePrepare3(virConnectPtr dconn, cookiein, cookieinlen, cookieout, cookieoutlen, uri_in, uri_out, - &def, origname, NULL, flags); + &def, origname, NULL, 0, NULL, flags); cleanup: VIR_FREE(origname); @@ -12462,6 +12477,8 @@ qemuDomainMigratePrepare
[libvirt] [PATCH v4 7/9] util: add virTypedParamsAddStringList
The `virTypedParamsAddStringList' function provides interface to add a NULL-terminated array of string values as a multi-value to the params. Signed-off-by: Pavel Boldin Signed-off-by: Michal Privoznik --- include/libvirt/libvirt-host.h | 6 ++ src/libvirt_public.syms| 1 + src/util/virtypedparam.c | 36 tests/virtypedparamtest.c | 28 4 files changed, 71 insertions(+) diff --git a/include/libvirt/libvirt-host.h b/include/libvirt/libvirt-host.h index 8222cfb..55a8e5d 100644 --- a/include/libvirt/libvirt-host.h +++ b/include/libvirt/libvirt-host.h @@ -331,6 +331,12 @@ virTypedParamsAddString (virTypedParameterPtr *params, const char *name, const char *value); int +virTypedParamsAddStringList(virTypedParameterPtr *params, + int *nparams, + int *maxparams, + const char *name, + const char **values); +int virTypedParamsAddFromString(virTypedParameterPtr *params, int *nparams, int *maxparams, diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 0a1feea..ccc7532 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -718,6 +718,7 @@ LIBVIRT_1.2.16 { LIBVIRT_1.3.0 { global: virTypedParamsGetAllStrings; +virTypedParamsAddStringList; } LIBVIRT_1.2.16; # define new API here using predicted next version number diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c index a12006c..ff2c878 100644 --- a/src/util/virtypedparam.c +++ b/src/util/virtypedparam.c @@ -1187,6 +1187,42 @@ virTypedParamsAddString(virTypedParameterPtr *params, return -1; } +/** + * virTypedParamsAddStringList: + * @params: array of typed parameters + * @nparams: number of parameters in the @params array + * @maxparams: maximum number of parameters that can be stored in @params + * array without allocating more memory + * @name: name of the parameter to store values to + * @values: the values to store into the new parameters + * + * Packs NULL-terminated list of strings @values into @params under the + * key @name. + * + * Returns 0 on success, -1 on error. + */ +int +virTypedParamsAddStringList(virTypedParameterPtr *params, +int *nparams, +int *maxparams, +const char *name, +const char **values) +{ +size_t i; +int rv = -1; + +if (!values) +return 0; + +for (i = 0; values[i]; i++) { +if ((rv = virTypedParamsAddString(params, nparams, maxparams, + name, values[i])) < 0) +break; +} + +return rv; +} + /** * virTypedParamsAddFromString: diff --git a/tests/virtypedparamtest.c b/tests/virtypedparamtest.c index 698255a..2869535 100644 --- a/tests/virtypedparamtest.c +++ b/tests/virtypedparamtest.c @@ -126,6 +126,31 @@ testTypedParamsFilter(const void *opaque ATTRIBUTE_UNUSED) } static int +testTypedParamsAddStringList(const void *opaque ATTRIBUTE_UNUSED) +{ +int rv = 0; +virTypedParameterPtr params = NULL; +int nparams = 0, maxparams = 0, i; + +const char *values[] = { +"foo", "bar", "foobar", NULL +}; + +rv = virTypedParamsAddStringList(¶ms, &nparams, &maxparams, "param", + values); + +for (i = 0; i < nparams; i++) { +if (STRNEQ(params[i].field, "param") || +STRNEQ(params[i].value.s, values[i]) || +params[i].type != VIR_TYPED_PARAM_STRING) +rv = -1; +} + +virTypedParamsFree(params, nparams); +return rv; +} + +static int testTypedParamsGetAllStrings(const void *opaque ATTRIBUTE_UNUSED) { int i, picked; @@ -259,6 +284,9 @@ mymain(void) if (virtTestRun("Get All Strings", testTypedParamsGetAllStrings, NULL) < 0) rv = -1; +if (virtTestRun("Add string list", testTypedParamsAddStringList, NULL) < 0) +rv = -1; + if (rv < 0) return EXIT_FAILURE; return EXIT_SUCCESS; -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4 1/9] virDomainDiskGetSource: Mark passed disk as 'const'
From: Michal Privoznik The disk is not changed anywhere in the function. Mark this fact in the function header too. Signed-off-by: Michal Privoznik --- src/conf/domain_conf.c | 2 +- src/conf/domain_conf.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 36de844..1f433ee 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1467,7 +1467,7 @@ virDomainDiskSetType(virDomainDiskDefPtr def, int type) const char * -virDomainDiskGetSource(virDomainDiskDefPtr def) +virDomainDiskGetSource(virDomainDiskDef const *def) { return def->src->path; } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index ba17a8d..bddf837 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2450,7 +2450,7 @@ void virDomainDiskDefFree(virDomainDiskDefPtr def); void virDomainLeaseDefFree(virDomainLeaseDefPtr def); int virDomainDiskGetType(virDomainDiskDefPtr def); void virDomainDiskSetType(virDomainDiskDefPtr def, int type); -const char *virDomainDiskGetSource(virDomainDiskDefPtr def); +const char *virDomainDiskGetSource(virDomainDiskDef const *def); int virDomainDiskSetSource(virDomainDiskDefPtr def, const char *src) ATTRIBUTE_RETURN_CHECK; const char *virDomainDiskGetDriver(virDomainDiskDefPtr def); -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4 5/9] util: multi-value parameters in virTypedParamsAdd*
Allow multi-value parameters to be build using virTypedParamsAdd* functions by removing check for duplicates. Signed-off-by: Pavel Boldin Signed-off-by: Michal Privoznik --- src/util/virtypedparam.c | 16 1 file changed, 16 deletions(-) diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c index 68620f5..6f608d6 100644 --- a/src/util/virtypedparam.c +++ b/src/util/virtypedparam.c @@ -749,14 +749,6 @@ virTypedParamsGetString(virTypedParameterPtr params, } -#define VIR_TYPED_PARAM_CHECK() \ -do { if (virTypedParamsGet(*params, n, name)) { \ -virReportError(VIR_ERR_INVALID_ARG, \ - _("Parameter '%s' is already set"), name); \ -goto error; \ -} } while (0) - - /** * virTypedParamsAddInt: * @params: pointer to the array of typed parameters @@ -787,7 +779,6 @@ virTypedParamsAddInt(virTypedParameterPtr *params, virResetLastError(); -VIR_TYPED_PARAM_CHECK(); if (VIR_RESIZE_N(*params, max, n, 1) < 0) goto error; *maxparams = max; @@ -835,7 +826,6 @@ virTypedParamsAddUInt(virTypedParameterPtr *params, virResetLastError(); -VIR_TYPED_PARAM_CHECK(); if (VIR_RESIZE_N(*params, max, n, 1) < 0) goto error; *maxparams = max; @@ -883,7 +873,6 @@ virTypedParamsAddLLong(virTypedParameterPtr *params, virResetLastError(); -VIR_TYPED_PARAM_CHECK(); if (VIR_RESIZE_N(*params, max, n, 1) < 0) goto error; *maxparams = max; @@ -931,7 +920,6 @@ virTypedParamsAddULLong(virTypedParameterPtr *params, virResetLastError(); -VIR_TYPED_PARAM_CHECK(); if (VIR_RESIZE_N(*params, max, n, 1) < 0) goto error; *maxparams = max; @@ -979,7 +967,6 @@ virTypedParamsAddDouble(virTypedParameterPtr *params, virResetLastError(); -VIR_TYPED_PARAM_CHECK(); if (VIR_RESIZE_N(*params, max, n, 1) < 0) goto error; *maxparams = max; @@ -1027,7 +1014,6 @@ virTypedParamsAddBoolean(virTypedParameterPtr *params, virResetLastError(); -VIR_TYPED_PARAM_CHECK(); if (VIR_RESIZE_N(*params, max, n, 1) < 0) goto error; *maxparams = max; @@ -1078,7 +1064,6 @@ virTypedParamsAddString(virTypedParameterPtr *params, virResetLastError(); -VIR_TYPED_PARAM_CHECK(); if (VIR_RESIZE_N(*params, max, n, 1) < 0) goto error; *maxparams = max; @@ -1136,7 +1121,6 @@ virTypedParamsAddFromString(virTypedParameterPtr *params, virResetLastError(); -VIR_TYPED_PARAM_CHECK(); if (VIR_RESIZE_N(*params, max, n, 1) < 0) goto error; *maxparams = max; -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4 0/9] Selective block device migration implementation
Behold of the fourth re-roll of the selective block device migration patch. In this patch we don't only fix the issue with NBD migration format auto- detection but also introduce the patches that do enhance the NBD migration triggered by `drive-mirror` QEMU command with ability for the user to select what disks are to be migrated based on the target name. First two patches fix some nitpicks, third one fixes the issue with NBD format auto-detection. Middle ones introduce a necessary API to keep a list of block devices to migrate in the virTypedParameter array and to work with this list. Of the two last patches first introduces the `migrate_disks' qemuMigration* parameter and pushes it down the call stack making the code to consult it when there is a decision to be made whether the block device is to be migrated to the new host. When there is no `migrate_disks' parameter given then the old scheme is used: only non-shared non-readonly disks with a source are migrated. The last patch promotes this ability up to the virsh utility and documents it as appropriate. Michal Privoznik (3): virDomainDiskGetSource: Mark passed disk as 'const' qemuMigrationBeginPhase: Fix function header indentation qemuMigrationDriveMirror: Force raw format for NBD Pavel Boldin (6): util: multi-value virTypedParameter util: multi-value parameters in virTypedParamsAdd* util: virTypedParams{Filter,GetAllStrings} util: add virTypedParamsAddStringList qemu: migration: selective block device migration virsh: selective block device migration include/libvirt/libvirt-domain.h | 9 ++ include/libvirt/libvirt-host.h | 11 ++ src/conf/domain_conf.c | 2 +- src/conf/domain_conf.h | 2 +- src/libvirt_public.syms | 6 + src/qemu/qemu_driver.c | 78 --- src/qemu/qemu_migration.c| 264 +-- src/qemu/qemu_migration.h| 24 ++-- src/util/virtypedparam.c | 259 +++--- src/util/virtypedparam.h | 19 +++ tests/Makefile.am| 6 + tests/virtypedparamtest.c| 295 +++ tools/virsh-domain.c | 23 +++ tools/virsh.pod | 21 +-- 14 files changed, 854 insertions(+), 165 deletions(-) create mode 100644 tests/virtypedparamtest.c -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 9/9] virsh: selective block device migration
Michal, Should I fix these or will you do it? Pavel On Fri, May 29, 2015 at 6:37 PM, John Ferlan wrote: > > > On 05/26/2015 09:01 AM, Michal Privoznik wrote: > > From: Pavel Boldin > > > > Add `virsh migrate' option `--migratedisks' that allows CLI user to > > explicitly specify block devices to migrate. > > > > Signed-off-by: Pavel Boldin > > Signed-off-by: Michal Privoznik > > --- > > tools/virsh-domain.c | 23 +++ > > tools/virsh.pod | 21 - > > 2 files changed, 35 insertions(+), 9 deletions(-) > > > > diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c > > index 91a1ca2..41d3829 100644 > > --- a/tools/virsh-domain.c > > +++ b/tools/virsh-domain.c > > @@ -9889,6 +9889,10 @@ static const vshCmdOptDef opts_migrate[] = { > > .type = VSH_OT_STRING, > > .help = N_("filename containing updated XML for the target") > > }, > > +{.name = "migratedisks", > > + .type = VSH_OT_STRING, > > + .help = N_("comma separated list of disks to be migrated") > > +}, > > {.name = NULL} > > }; > > > > @@ -9948,6 +9952,25 @@ doMigrate(void *opaque) > > VIR_MIGRATE_PARAM_DEST_NAME, opt) < 0) > > goto save_error; > > > > +if (vshCommandOptStringReq(ctl, cmd, "migratedisks", &opt) < 0) > > +goto out; > > +if (opt) { > > +char **val = NULL; > > + > > +val = virStringSplit(opt, ",", 0); > > + > > +if (virTypedParamsAddStringList(¶ms, > > +&nparams, > > +&maxparams, > > +VIR_MIGRATE_PARAM_MIGRATE_DISKS, > > +(const char **)val) < 0) { > > +VIR_FREE(val); > > +goto save_error; > > +} > > + > > +VIR_FREE(val); > > +} > > + > > if (vshCommandOptStringReq(ctl, cmd, "xml", &opt) < 0) > > goto out; > > if (opt) { > > diff --git a/tools/virsh.pod b/tools/virsh.pod > > index d588e5a..9275091 100644 > > --- a/tools/virsh.pod > > +++ b/tools/virsh.pod > > @@ -1521,6 +1521,7 @@ to the I namespace is displayed instead of > being modified. > > [I<--compressed>] [I<--abort-on-error>] [I<--auto-converge>] > > I I [I] [I] > [I] > > [I] [I<--timeout> B] [I<--xml> B] > > +[I<--migratedisks> B] > > > > Migrate domain to another host. Add I<--live> for live migration; > <--p2p> > > for peer-2-peer migration; I<--direct> for direct migration; or > I<--tunnelled> > > @@ -1536,15 +1537,17 @@ with incremental copy (same base image shared > between source and destination). > > In both cases the disk images have to exist on destination host, the > > I<--copy-storage-...> options only tell libvirt to transfer data from > the > > images on source host to the images found at the same place on the > destination > > -host. I<--change-protection> enforces that no incompatible configuration > > -changes will be made to the domain while the migration is underway; > this flag > > -is implicitly enabled when supported by the hypervisor, but can be > explicitly > > -used to reject the migration if the hypervisor lacks change protection > > -support. I<--verbose> displays the progress of migration. > I<--compressed> > > -activates compression of memory pages that have to be transferred > repeatedly > > -during live migration. I<--abort-on-error> cancels the migration if a > soft > > -error (for example I/O error) happens during the migration. > I<--auto-converge> > > -forces convergence during live migration. > > +host. By default only non-shared non-readonly images are transfered. Use > > +I<--migratedisks> to explicitly specify a list of disk targets to > transfer. > > +B is a comma separated list then. I<--change-protection> > enforces > > +that no incompatible configuration changes will be made to the domain > while the > > +migration is underway; this flag is implicitly enabled when supported > by the > > +hypervisor, but can be explicitly used to reject the migration if the > > +hypervisor lacks change protection support. I<--verbose> displays the > progress > > +of migration.
Re: [libvirt] [PATCHv2 0/6] Selective block device migration implementation
Thank you. I will go through your comments just in case I'm going to push any other changes. Sorry for the inconvenience from my patches :-/. Pavel On Mon, May 25, 2015 at 5:58 PM, Michal Privoznik wrote: > On 21.05.2015 13:07, Pavel Boldin wrote: > > The patchset represented in the mail thread implements the selective > block > > device migration for the QEMU driver. This closes the referenced bug [1]. > > > > First the supplementary API implemented allowing for multiple key-values > pair > > in the virTypedParameter arrays. This is used to pass the list of block > > devices to migrate in the following patches. Unit tests for this are > added as > > well. This is the subject of the first four patches. > > > > Fifth patch is adding the necessary parameter `migrate_disks' and passes > it > > through the QEMU driver call stack. The introduced `qemuMigrateDisk' > function > > is then checks that the disk is to be migrated because it is in the > list. If > > there is no list specified then the legacy check is used: the device is > migrate > > if it is not shared, not readonly and has a source. > > > > Sixth and the last patch is adding the necessary code to the `virsh' > utility > > making it able for user to specify a comma-separated list of the block > device > > names that are to be migrated. > > > > The implemented Python bindings patch is to be presented. > > > > Changes in v2: > > * Addressed review comments > > * Removed all the duplicates check in the commit 'multi-value > parameters in > > virTypedParamsAdd*' > > * reimplemented virTypedParamsPick as virTypedParamsFilter > > * renamed virTypedParamsPackStrings to virTypedParamsAddStringList > > > > [1] https://bugzilla.redhat.com/show_bug.cgi?id=1203032 > > > > Pavel Boldin (6): > > util: multi-value virTypedParameter > > util: multi-value parameters in virTypedParamsAdd* > > util: virTypedParams{Filter,PickStrings} > > util: add virTypedParamsAddStringList > > qemu: migration: selective block device migration > > virsh: selective block device migration > > > > include/libvirt/libvirt-domain.h | 9 ++ > > include/libvirt/libvirt-host.h | 16 +++ > > src/libvirt_public.syms | 7 + > > src/qemu/qemu_driver.c | 66 ++--- > > src/qemu/qemu_migration.c| 174 +++ > > src/qemu/qemu_migration.h| 23 ++-- > > src/util/virtypedparam.c | 263 > --- > > src/util/virtypedparam.h | 10 ++ > > tests/Makefile.am| 6 + > > tests/virtypedparamtest.c| 291 > +++ > > tools/virsh-domain.c | 23 > > tools/virsh.pod | 5 +- > > 12 files changed, 750 insertions(+), 143 deletions(-) > > create mode 100644 tests/virtypedparamtest.c > > > > So, I think we are almost there. I've pointed out a few things. > Moreover, as I've been reviewing I keep fixing the nits I'm raising. So > instead of me giving you headache of posting v3, I'll do that. > > Michal > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 6/6] virsh: selective block device migration
Add `virsh migrate' option `--migratedisks' that allows CLI user to explicitly specify block devices to migrate. Signed-off-by: Pavel Boldin --- tools/virsh-domain.c | 23 +++ tools/virsh.pod | 5 - 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 20f8c75..47a24ab 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -9784,6 +9784,10 @@ static const vshCmdOptDef opts_migrate[] = { .type = VSH_OT_STRING, .help = N_("filename containing updated XML for the target") }, +{.name = "migratedisks", + .type = VSH_OT_STRING, + .help = N_("comma separated list of disks to be migrated") +}, {.name = NULL} }; @@ -9843,6 +9847,25 @@ doMigrate(void *opaque) VIR_MIGRATE_PARAM_DEST_NAME, opt) < 0) goto save_error; +if (vshCommandOptStringReq(ctl, cmd, "migratedisks", &opt) < 0) +goto out; +if (opt) { +char **val = NULL; + +val = virStringSplit(opt, ",", 0); + +if (virTypedParamsAddStringList(¶ms, +&nparams, +&maxparams, +VIR_MIGRATE_PARAM_MIGRATE_DISKS, +(const char **)val) < 0) { +VIR_FREE(val); +goto save_error; +} + +VIR_FREE(val); +} + if (vshCommandOptStringReq(ctl, cmd, "xml", &opt) < 0) goto out; if (opt) { diff --git a/tools/virsh.pod b/tools/virsh.pod index 1bb655b..aad0f3b 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1521,6 +1521,7 @@ to the I namespace is displayed instead of being modified. [I<--compressed>] [I<--abort-on-error>] [I<--auto-converge>] I I [I] [I] [I] [I] [I<--timeout> B] [I<--xml> B] +[I<--migratedisks> B] Migrate domain to another host. Add I<--live> for live migration; <--p2p> for peer-2-peer migration; I<--direct> for direct migration; or I<--tunnelled> @@ -1536,7 +1537,9 @@ with incremental copy (same base image shared between source and destination). In both cases the disk images have to exist on destination host, the I<--copy-storage-...> options only tell libvirt to transfer data from the images on source host to the images found at the same place on the destination -host. I<--change-protection> enforces that no incompatible configuration +host. By default only non-shared non-readonly images are transfered. Use +I<--migratedisks> to explicitly specify a list of images to transfer. +I<--change-protection> enforces that no incompatible configuration changes will be made to the domain while the migration is underway; this flag is implicitly enabled when supported by the hypervisor, but can be explicitly used to reject the migration if the hypervisor lacks change protection -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 5/6] qemu: migration: selective block device migration
Implement a `migrate_disks' parameters for the QEMU driver. This multi- value parameter can be used to explicitly specify what block devices are to be migrated using the NBD server. Tunnelled migration using NBD is to be done. Signed-off-by: Pavel Boldin --- include/libvirt/libvirt-domain.h | 9 ++ src/qemu/qemu_driver.c | 66 ++- src/qemu/qemu_migration.c| 174 +-- src/qemu/qemu_migration.h| 23 -- 4 files changed, 183 insertions(+), 89 deletions(-) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 0f465b9..6b48923 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -748,6 +748,15 @@ typedef enum { */ # define VIR_MIGRATE_PARAM_LISTEN_ADDRESS"listen_address" +/** + * VIR_MIGRATE_PARAM_MIGRATE_DISKS: + * + * virDomainMigrate* params multiple field: The multiple values that list + * the block devices to be migrated. At the moment this is only supported + * by the QEMU driver but not for the tunnelled migration. + */ +# define VIR_MIGRATE_PARAM_MIGRATE_DISKS"migrate_disks" + /* Domain migration. */ virDomainPtr virDomainMigrate (virDomainPtr domain, virConnectPtr dconn, unsigned long flags, const char *dname, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2668011..77b7d9d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12472,7 +12472,7 @@ qemuDomainMigratePrepare2(virConnectPtr dconn, ret = qemuMigrationPrepareDirect(driver, dconn, NULL, 0, NULL, NULL, /* No cookies */ uri_in, uri_out, - &def, origname, NULL, flags); + &def, origname, NULL, flags, NULL); cleanup: VIR_FREE(origname); @@ -12525,7 +12525,7 @@ qemuDomainMigratePerform(virDomainPtr dom, * Consume any cookie we were able to decode though */ ret = qemuMigrationPerform(driver, dom->conn, vm, - NULL, dconnuri, uri, NULL, NULL, + NULL, dconnuri, uri, NULL, NULL, NULL, cookie, cookielen, NULL, NULL, /* No output cookies in v2 */ flags, dname, resource, false); @@ -12602,7 +12602,7 @@ qemuDomainMigrateBegin3(virDomainPtr domain, } return qemuMigrationBegin(domain->conn, vm, xmlin, dname, - cookieout, cookieoutlen, flags); + cookieout, cookieoutlen, flags, NULL); } static char * @@ -12615,11 +12615,13 @@ qemuDomainMigrateBegin3Params(virDomainPtr domain, { const char *xmlin = NULL; const char *dname = NULL; +const char **migrate_disks = NULL; +char *ret = NULL; virDomainObjPtr vm; virCheckFlags(QEMU_MIGRATION_FLAGS, NULL); if (virTypedParamsValidate(params, nparams, QEMU_MIGRATION_PARAMETERS) < 0) -return NULL; +goto cleanup; if (virTypedParamsGetString(params, nparams, VIR_MIGRATE_PARAM_DEST_XML, @@ -12627,18 +12629,26 @@ qemuDomainMigrateBegin3Params(virDomainPtr domain, virTypedParamsGetString(params, nparams, VIR_MIGRATE_PARAM_DEST_NAME, &dname) < 0) -return NULL; +goto cleanup; + +migrate_disks = virTypedParamsPickStrings(params, nparams, + VIR_MIGRATE_PARAM_MIGRATE_DISKS, + NULL); if (!(vm = qemuDomObjFromDomain(domain))) -return NULL; +goto cleanup; if (virDomainMigrateBegin3ParamsEnsureACL(domain->conn, vm->def) < 0) { virDomainObjEndAPI(&vm); -return NULL; +goto cleanup; } -return qemuMigrationBegin(domain->conn, vm, xmlin, dname, - cookieout, cookieoutlen, flags); +ret = qemuMigrationBegin(domain->conn, vm, xmlin, dname, + cookieout, cookieoutlen, flags, migrate_disks); + + cleanup: +VIR_FREE(migrate_disks); +return ret; } @@ -12682,7 +12692,8 @@ qemuDomainMigratePrepare3(virConnectPtr dconn, cookiein, cookieinlen, cookieout, cookieoutlen, uri_in, uri_out, - &def, origname, NULL, flags); + &def, origname, NULL, flags, + NULL); cleanup: VIR_FREE(origname); @@ -12708,6 +12719,7 @@ qemuDomainMigratePrepare3Params(virConnectPtr dconn, const char *dname = NULL; const char *uri_in = N
[libvirt] [PATCH v2 1/6] util: multi-value virTypedParameter
The `virTypedParamsValidate' function now can be instructed to allow multiple entries for some of the keys. For this flag the type with the `VIR_TYPED_PARAM_MULTIPLE' flag. Add unit tests for this new behaviour. Signed-off-by: Pavel Boldin --- src/util/virtypedparam.c | 108 +++--- src/util/virtypedparam.h | 10 +++ tests/Makefile.am | 6 ++ tests/virtypedparamtest.c | 167 ++ 4 files changed, 252 insertions(+), 39 deletions(-) create mode 100644 tests/virtypedparamtest.c diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c index de2d447..43e49ca 100644 --- a/src/util/virtypedparam.c +++ b/src/util/virtypedparam.c @@ -47,11 +47,18 @@ VIR_ENUM_IMPL(virTypedParameter, VIR_TYPED_PARAM_LAST, * internal utility functions (those in libvirt_private.syms) may * report errors that the caller will dispatch. */ +static int virTypedParamsSortName(const void *left, const void *right) +{ +const virTypedParameter *param_left = left, *param_right = right; +return strcmp(param_left->field, param_right->field); +} + /* Validate that PARAMS contains only recognized parameter names with - * correct types, and with no duplicates. Pass in as many name/type - * pairs as appropriate, and pass NULL to end the list of accepted - * parameters. Return 0 on success, -1 on failure with error message - * already issued. */ + * correct types, and with no duplicates except for parameters + * specified with VIR_TYPED_PARAM_MULTIPLE flag in type. + * Pass in as many name/type pairs as appropriate, and pass NULL to end + * the list of accepted parameters. Return 0 on success, -1 on failure + * with error message already issued. */ int virTypedParamsValidate(virTypedParameterPtr params, int nparams, ...) { @@ -60,60 +67,83 @@ virTypedParamsValidate(virTypedParameterPtr params, int nparams, ...) size_t i, j; const char *name; int type; +size_t nkeys = 0, nkeysmax = 0; +virTypedParameterPtr sorted = NULL, keys = NULL; va_start(ap, nparams); -/* Yes, this is quadratic, but since we reject duplicates and - * unknowns, it is constrained by the number of var-args passed - * in, which is expected to be small enough to not be - * noticeable. */ -for (i = 0; i < nparams; i++) { -va_end(ap); -va_start(ap, nparams); +if (VIR_ALLOC_N(sorted, nparams) < 0) +goto cleanup; -name = va_arg(ap, const char *); -while (name) { -type = va_arg(ap, int); -if (STREQ(params[i].field, name)) { -if (params[i].type != type) { -const char *badtype; - -badtype = virTypedParameterTypeToString(params[i].type); -if (!badtype) -badtype = virTypedParameterTypeToString(0); -virReportError(VIR_ERR_INVALID_ARG, - _("invalid type '%s' for parameter '%s', " - "expected '%s'"), - badtype, params[i].field, - virTypedParameterTypeToString(type)); -} -break; -} -name = va_arg(ap, const char *); -} -if (!name) { -virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, - _("parameter '%s' not supported"), - params[i].field); +/* Here we intentionally don't copy values */ +memcpy(sorted, params, sizeof(*params) * nparams); +qsort(sorted, nparams, sizeof(*sorted), virTypedParamsSortName); + +name = va_arg(ap, const char *); +while (name) { +type = va_arg(ap, int); +if (VIR_RESIZE_N(keys, nkeysmax, nkeys, 1) < 0) +goto cleanup; + +if (virStrcpyStatic(keys[nkeys].field, name) == NULL) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _("Field name '%s' too long"), name); goto cleanup; } -for (j = 0; j < i; j++) { -if (STREQ(params[i].field, params[j].field)) { + +keys[nkeys].type = type & ~VIR_TYPED_PARAM_MULTIPLE; +/* Value is not used anyway */ +keys[nkeys].value.i = type & VIR_TYPED_PARAM_MULTIPLE; + +nkeys++; +name = va_arg(ap, const char *); +} + +qsort(keys, nkeys, sizeof(*keys), virTypedParamsSortName); + +for (i = 0, j = 0; i < nparams && j < nkeys;) { +if (STRNEQ(sorted[i].field, keys[j].field)) { +j++; +} else { +if (i > j && !(keys[j].value.i & VIR_TYPED_PARAM_MULTIPLE)) { virReportError(VIR_ERR_INVALID_ARG, _("para
[libvirt] [PATCH v2 4/6] util: add virTypedParamsAddStringList
The `virTypedParamsAddStringList' function provides interface to add a NULL-terminated array of string values as a multi-value to the params. Signed-off-by: Pavel Boldin --- include/libvirt/libvirt-host.h | 6 ++ src/libvirt_public.syms| 1 + src/util/virtypedparam.c | 36 tests/virtypedparamtest.c | 28 4 files changed, 71 insertions(+) diff --git a/include/libvirt/libvirt-host.h b/include/libvirt/libvirt-host.h index 36267fc..7d0b5b7 100644 --- a/include/libvirt/libvirt-host.h +++ b/include/libvirt/libvirt-host.h @@ -336,6 +336,12 @@ virTypedParamsAddString (virTypedParameterPtr *params, const char *name, const char *value); int +virTypedParamsAddStringList(virTypedParameterPtr *params, + int *nparams, + int *maxparams, + const char *name, + const char **values); +int virTypedParamsAddFromString(virTypedParameterPtr *params, int *nparams, int *maxparams, diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 8fc8c42..4de23c0 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -714,6 +714,7 @@ LIBVIRT_1.2.16 { global: virTypedParamsFilter; virTypedParamsPickStrings; +virTypedParamsAddStringList; } LIBVIRT_1.2.15; # define new API here using predicted next version number diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c index a3d959e..a2e99ee 100644 --- a/src/util/virtypedparam.c +++ b/src/util/virtypedparam.c @@ -1191,6 +1191,42 @@ virTypedParamsAddString(virTypedParameterPtr *params, return -1; } +/** + * virTypedParamsAddStringList: + * @params: array of typed parameters + * @nparams: number of parameters in the @params array + * @maxparams: maximum number of parameters that can be stored in @params + * array without allocating more memory + * @name: name of the parameter to store values to + * @values: the values to store into the new parameters + * + * Packs NULL-terminated list of strings @values into @params under the + * key @name. + * + * Returns 0 on success, -1 on error. + */ +int +virTypedParamsAddStringList(virTypedParameterPtr *params, +int *nparams, +int *maxparams, +const char *name, +const char **values) +{ +size_t i; +int rv = -1; + +if (!values) +return 0; + +for (i = 0; values[i]; i++) { +if ((rv = virTypedParamsAddString(params, nparams, maxparams, + name, values[i])) < 0) +break; +} + +return rv; +} + /** * virTypedParamsAddFromString: diff --git a/tests/virtypedparamtest.c b/tests/virtypedparamtest.c index 945dbe7..26fa4b2 100644 --- a/tests/virtypedparamtest.c +++ b/tests/virtypedparamtest.c @@ -126,6 +126,31 @@ testTypedParamsFilter(const void *opaque ATTRIBUTE_UNUSED) } static int +testTypedParamsAddStringList(const void *opaque ATTRIBUTE_UNUSED) +{ +int rv = 0; +virTypedParameterPtr params = NULL; +int nparams = 0, maxparams = 0, i; + +const char *values[] = { +"foo", "bar", "foobar", NULL +}; + +rv = virTypedParamsAddStringList(¶ms, &nparams, &maxparams, "param", + values); + +for (i = 0; i < nparams; i++) { +if (STRNEQ(params[i].field, "param") || +STRNEQ(params[i].value.s, values[i]) || +params[i].type != VIR_TYPED_PARAM_STRING) +rv = -1; +} + +virTypedParamsFree(params, nparams); +return rv; +} + +static int testTypedParamsPickStrings(const void *opaque ATTRIBUTE_UNUSED) { size_t i, picked; @@ -255,6 +280,9 @@ mymain(void) if (virtTestRun("Picking Strings", testTypedParamsPickStrings, NULL) < 0) rv = -1; +if (virtTestRun("Add string list", testTypedParamsAddStringList, NULL) < 0) +rv = -1; + if (rv < 0) return EXIT_FAILURE; return EXIT_SUCCESS; -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 0/6] Selective block device migration implementation
The patchset represented in the mail thread implements the selective block device migration for the QEMU driver. This closes the referenced bug [1]. First the supplementary API implemented allowing for multiple key-values pair in the virTypedParameter arrays. This is used to pass the list of block devices to migrate in the following patches. Unit tests for this are added as well. This is the subject of the first four patches. Fifth patch is adding the necessary parameter `migrate_disks' and passes it through the QEMU driver call stack. The introduced `qemuMigrateDisk' function is then checks that the disk is to be migrated because it is in the list. If there is no list specified then the legacy check is used: the device is migrate if it is not shared, not readonly and has a source. Sixth and the last patch is adding the necessary code to the `virsh' utility making it able for user to specify a comma-separated list of the block device names that are to be migrated. The implemented Python bindings patch is to be presented. Changes in v2: * Addressed review comments * Removed all the duplicates check in the commit 'multi-value parameters in virTypedParamsAdd*' * reimplemented virTypedParamsPick as virTypedParamsFilter * renamed virTypedParamsPackStrings to virTypedParamsAddStringList [1] https://bugzilla.redhat.com/show_bug.cgi?id=1203032 Pavel Boldin (6): util: multi-value virTypedParameter util: multi-value parameters in virTypedParamsAdd* util: virTypedParams{Filter,PickStrings} util: add virTypedParamsAddStringList qemu: migration: selective block device migration virsh: selective block device migration include/libvirt/libvirt-domain.h | 9 ++ include/libvirt/libvirt-host.h | 16 +++ src/libvirt_public.syms | 7 + src/qemu/qemu_driver.c | 66 ++--- src/qemu/qemu_migration.c| 174 +++ src/qemu/qemu_migration.h| 23 ++-- src/util/virtypedparam.c | 263 --- src/util/virtypedparam.h | 10 ++ tests/Makefile.am| 6 + tests/virtypedparamtest.c| 291 +++ tools/virsh-domain.c | 23 tools/virsh.pod | 5 +- 12 files changed, 750 insertions(+), 143 deletions(-) create mode 100644 tests/virtypedparamtest.c -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 2/6] util: multi-value parameters in virTypedParamsAdd*
Allow multi-value parameters to be build using virTypedParamsAdd* functions by removing check for duplicates. Signed-off-by: Pavel Boldin --- src/util/virtypedparam.c | 16 1 file changed, 16 deletions(-) diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c index 43e49ca..ec20b74 100644 --- a/src/util/virtypedparam.c +++ b/src/util/virtypedparam.c @@ -748,14 +748,6 @@ virTypedParamsGetString(virTypedParameterPtr params, } -#define VIR_TYPED_PARAM_CHECK() \ -do { if (virTypedParamsGet(*params, n, name)) { \ -virReportError(VIR_ERR_INVALID_ARG, \ - _("Parameter '%s' is already set"), name); \ -goto error; \ -} } while (0) - - /** * virTypedParamsAddInt: * @params: pointer to the array of typed parameters @@ -786,7 +778,6 @@ virTypedParamsAddInt(virTypedParameterPtr *params, virResetLastError(); -VIR_TYPED_PARAM_CHECK(); if (VIR_RESIZE_N(*params, max, n, 1) < 0) goto error; *maxparams = max; @@ -834,7 +825,6 @@ virTypedParamsAddUInt(virTypedParameterPtr *params, virResetLastError(); -VIR_TYPED_PARAM_CHECK(); if (VIR_RESIZE_N(*params, max, n, 1) < 0) goto error; *maxparams = max; @@ -882,7 +872,6 @@ virTypedParamsAddLLong(virTypedParameterPtr *params, virResetLastError(); -VIR_TYPED_PARAM_CHECK(); if (VIR_RESIZE_N(*params, max, n, 1) < 0) goto error; *maxparams = max; @@ -930,7 +919,6 @@ virTypedParamsAddULLong(virTypedParameterPtr *params, virResetLastError(); -VIR_TYPED_PARAM_CHECK(); if (VIR_RESIZE_N(*params, max, n, 1) < 0) goto error; *maxparams = max; @@ -978,7 +966,6 @@ virTypedParamsAddDouble(virTypedParameterPtr *params, virResetLastError(); -VIR_TYPED_PARAM_CHECK(); if (VIR_RESIZE_N(*params, max, n, 1) < 0) goto error; *maxparams = max; @@ -1026,7 +1013,6 @@ virTypedParamsAddBoolean(virTypedParameterPtr *params, virResetLastError(); -VIR_TYPED_PARAM_CHECK(); if (VIR_RESIZE_N(*params, max, n, 1) < 0) goto error; *maxparams = max; @@ -1077,7 +1063,6 @@ virTypedParamsAddString(virTypedParameterPtr *params, virResetLastError(); -VIR_TYPED_PARAM_CHECK(); if (VIR_RESIZE_N(*params, max, n, 1) < 0) goto error; *maxparams = max; @@ -1135,7 +1120,6 @@ virTypedParamsAddFromString(virTypedParameterPtr *params, virResetLastError(); -VIR_TYPED_PARAM_CHECK(); if (VIR_RESIZE_N(*params, max, n, 1) < 0) goto error; *maxparams = max; -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 3/6] util: virTypedParams{Filter,PickStrings}
Add multikey API: * virTypedParamsFilter that returns all the parameters with specified name. * virTypedParamsPickStrings that returns a NULL-terminated `const char**' list with all the values for specified name and string type. Signed-off-by: Pavel Boldin --- include/libvirt/libvirt-host.h | 10 src/libvirt_public.syms| 6 +++ src/util/virtypedparam.c | 107 + tests/virtypedparamtest.c | 96 4 files changed, 219 insertions(+) diff --git a/include/libvirt/libvirt-host.h b/include/libvirt/libvirt-host.h index 070550b..36267fc 100644 --- a/include/libvirt/libvirt-host.h +++ b/include/libvirt/libvirt-host.h @@ -249,6 +249,11 @@ virTypedParamsGet (virTypedParameterPtr params, int nparams, const char *name); int +virTypedParamsFilter(virTypedParameterPtr params, + int nparams, + const char *name, + virTypedParameterPtr **ret); +int virTypedParamsGetInt(virTypedParameterPtr params, int nparams, const char *name, @@ -283,6 +288,11 @@ virTypedParamsGetString (virTypedParameterPtr params, int nparams, const char *name, const char **value); +const char ** +virTypedParamsPickStrings(virTypedParameterPtr params, + int nparams, + const char *name, + size_t *picked); int virTypedParamsAddInt(virTypedParameterPtr *params, int *nparams, diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index ef3d2f0..8fc8c42 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -710,4 +710,10 @@ LIBVIRT_1.2.15 { virDomainDelIOThread; } LIBVIRT_1.2.14; +LIBVIRT_1.2.16 { +global: +virTypedParamsFilter; +virTypedParamsPickStrings; +} LIBVIRT_1.2.15; + # define new API here using predicted next version number diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c index ec20b74..a3d959e 100644 --- a/src/util/virtypedparam.c +++ b/src/util/virtypedparam.c @@ -481,6 +481,58 @@ virTypedParamsGet(virTypedParameterPtr params, } +/** + * virTypedParamsFilter: + * @params: array of typed parameters + * @nparams: number of parameters in the @params array + * @name: name of the parameter to find + * @ret: pointer to the returned array + * + * + * Filters @params retaining only the parameters named @name in the + * resulting array @ret. Caller should free the @ret array but no the + * items since they are pointing to the @params elements. + * + * Returns amount of elements in @ret on success, -1 on error. + */ +int +virTypedParamsFilter(virTypedParameterPtr params, + int nparams, + const char *name, + virTypedParameterPtr **ret) +{ +size_t i, max = 0, n = 0; + +virResetLastError(); + +if (!params || !name || !ret) { +virReportError(VIR_ERR_INVALID_ARG, + _("Required argument is missing: %s"), + !params ? "params" : !name ? "name" : "ret"); +goto error; +} + +for (i = 0; i < nparams; i++) { +if (STREQ(params[i].field, name)) { +if (VIR_RESIZE_N(*ret, max, n, 1) < 0) +goto error; + +(*ret)[n] = ¶ms[i]; + +n++; +} +} + +return n; + + error: +if (ret) +VIR_FREE(*ret); +virDispatchError(NULL); +return -1; +} + + #define VIR_TYPED_PARAM_CHECK_TYPE(check_type) \ do { if (param->type != check_type) { \ virReportError(VIR_ERR_INVALID_ARG, \ @@ -749,6 +801,61 @@ virTypedParamsGetString(virTypedParameterPtr params, /** + * virTypedParamsPickStrings: + * @params: array of typed parameters + * @nparams: number of parameters in the @params array + * @name: name of the parameter to find + * @picked: pointer to the amount of picked strings. + * + * + * Finds typed parameters called @name. + * + * Returns a string list, which is a NULL terminated array of pointers to + * strings. Since a NULL is a valid parameter string value caller can ask + * for exact amount of picked strings using @picked argument. + * + * Caller should free the returned array but not the items since they are + * taken from @params array. + */ +const char ** +virTypedParamsPickStrings(virTypedParameterPtr params, + int nparams, const char *name, + size_t *picked) +{ +const char **values = NULL; +size_t i, n; +int nfiltered; +virTypedParameterPtr *filtere
[libvirt] [PATCH 1/5] util: multi-value virTypedParameter
The `virTypedParamsValidate' function now can be instructed to allow multiple entries for some of the keys. For this flag the type with the `VIR_TYPED_PARAM_MULTIPLE' flag. Add unit tests for this new behaviour. Signed-off-by: Pavel Boldin --- include/libvirt/libvirt-host.h | 8 ++ src/util/virtypedparam.c | 107 - tests/Makefile.am | 6 ++ tests/virtypedparamtest.c | 177 + 4 files changed, 259 insertions(+), 39 deletions(-) create mode 100644 tests/virtypedparamtest.c diff --git a/include/libvirt/libvirt-host.h b/include/libvirt/libvirt-host.h index 953366b..a3e8b88 100644 --- a/include/libvirt/libvirt-host.h +++ b/include/libvirt/libvirt-host.h @@ -180,6 +180,14 @@ typedef enum { } virTypedParameterType; /** + * VIR_TYPED_PARAM_MULTIPLE: + * + * Flag indiciating that the params has multiple occurences of the parameter. + * Only used as a flag for @type argument of the virTypedParamsValidate. + */ +# define VIR_TYPED_PARAM_MULTIPLE (1 << 31) + +/** * virTypedParameterFlags: * * Flags related to libvirt APIs that use virTypedParameter. diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c index de2d447..bd47609 100644 --- a/src/util/virtypedparam.c +++ b/src/util/virtypedparam.c @@ -47,11 +47,18 @@ VIR_ENUM_IMPL(virTypedParameter, VIR_TYPED_PARAM_LAST, * internal utility functions (those in libvirt_private.syms) may * report errors that the caller will dispatch. */ +static int _virTypedParamsSortName(const void *left, const void *right) +{ +const virTypedParameter *param_left = left, *param_right = right; +return strcmp(param_left->field, param_right->field); +} + /* Validate that PARAMS contains only recognized parameter names with - * correct types, and with no duplicates. Pass in as many name/type - * pairs as appropriate, and pass NULL to end the list of accepted - * parameters. Return 0 on success, -1 on failure with error message - * already issued. */ + * correct types, and with no duplicates except for parameters + * specified with VIR_TYPED_PARAM_MULTIPLE flag in type. + * Pass in as many name/type pairs as appropriate, and pass NULL to end + * the list of accepted parameters. Return 0 on success, -1 on failure + * with error message already issued. */ int virTypedParamsValidate(virTypedParameterPtr params, int nparams, ...) { @@ -60,60 +67,82 @@ virTypedParamsValidate(virTypedParameterPtr params, int nparams, ...) size_t i, j; const char *name; int type; +size_t nkeys = 0, nkeysmax = 0; +virTypedParameterPtr sorted = NULL, keys = NULL; va_start(ap, nparams); -/* Yes, this is quadratic, but since we reject duplicates and - * unknowns, it is constrained by the number of var-args passed - * in, which is expected to be small enough to not be - * noticeable. */ -for (i = 0; i < nparams; i++) { -va_end(ap); -va_start(ap, nparams); +if (VIR_ALLOC_N(sorted, nparams) < 0) +goto cleanup; -name = va_arg(ap, const char *); -while (name) { -type = va_arg(ap, int); -if (STREQ(params[i].field, name)) { -if (params[i].type != type) { -const char *badtype; - -badtype = virTypedParameterTypeToString(params[i].type); -if (!badtype) -badtype = virTypedParameterTypeToString(0); -virReportError(VIR_ERR_INVALID_ARG, - _("invalid type '%s' for parameter '%s', " - "expected '%s'"), - badtype, params[i].field, - virTypedParameterTypeToString(type)); -} -break; -} -name = va_arg(ap, const char *); -} -if (!name) { -virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, - _("parameter '%s' not supported"), - params[i].field); +/* Here we intentially don't copy values */ +memcpy(sorted, params, sizeof(*params) * nparams); +qsort(sorted, nparams, sizeof(*sorted), _virTypedParamsSortName); + +name = va_arg(ap, const char *); +while (name) { +type = va_arg(ap, int); +if (VIR_RESIZE_N(keys, nkeysmax, nkeys, 1) < 0) +goto cleanup; + +if (virStrcpyStatic(keys[nkeys].field, name) == NULL) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _("Field name '%s' too long"), name); goto cleanup; } -for (j = 0; j < i; j++) { -if (STREQ(params[i].field, params[j].field)) { + +keys[nkeys].type = type & ~VIR_TYPED_PARAM_MULTIPLE; +
[libvirt] [PATCH 3/5] util: add virTypedParamsPackStrings
The `virTypedParamsPackStrings' function provides interface to pack multiple string values under the same key to the `virTypedParameter' array. Signed-off-by: Pavel Boldin --- include/libvirt/libvirt-host.h | 6 +++ src/libvirt_public.syms| 1 + src/util/virtypedparam.c | 94 +- tests/virtypedparamtest.c | 28 + 4 files changed, 109 insertions(+), 20 deletions(-) diff --git a/include/libvirt/libvirt-host.h b/include/libvirt/libvirt-host.h index afa730f..090ac83 100644 --- a/include/libvirt/libvirt-host.h +++ b/include/libvirt/libvirt-host.h @@ -344,6 +344,12 @@ virTypedParamsAddString (virTypedParameterPtr *params, const char *name, const char *value); int +virTypedParamsPackStrings(virTypedParameterPtr *params, + int *nparams, + int *maxparams, + const char *name, + const char **values); +int virTypedParamsAddFromString(virTypedParameterPtr *params, int *nparams, int *maxparams, diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index d886967..8a24bb6 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -714,6 +714,7 @@ LIBVIRT_1.2.16 { global: virTypedParamsPick; virTypedParamsPickStrings; +virTypedParamsPackStrings; } LIBVIRT_1.2.15; # define new API here using predicted next version number diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c index 643d10e..9f2ab3c 100644 --- a/src/util/virtypedparam.c +++ b/src/util/virtypedparam.c @@ -1132,6 +1132,43 @@ virTypedParamsAddBoolean(virTypedParameterPtr *params, return -1; } +static int +virTypedParamsAddStringFull(virTypedParameterPtr *params, +int *nparams, +int *maxparams, +const char *name, +const char *value, +bool unique) +{ +char *str = NULL; +size_t max = *maxparams; +size_t n = *nparams; + +virResetLastError(); + +if (unique) +VIR_TYPED_PARAM_CHECK(); +if (VIR_RESIZE_N(*params, max, n, 1) < 0) +goto error; +*maxparams = max; + +if (VIR_STRDUP(str, value) < 0) +goto error; + +if (virTypedParameterAssign(*params + n, name, +VIR_TYPED_PARAM_STRING, str) < 0) { +VIR_FREE(str); +goto error; +} + +*nparams += 1; +return 0; + + error: +virDispatchError(NULL); +return -1; +} + /** * virTypedParamsAddString: @@ -1160,32 +1197,49 @@ virTypedParamsAddString(virTypedParameterPtr *params, const char *name, const char *value) { -char *str = NULL; -size_t max = *maxparams; -size_t n = *nparams; +return virTypedParamsAddStringFull(params, + nparams, + maxparams, + name, + value, + 1); +} -virResetLastError(); -VIR_TYPED_PARAM_CHECK(); -if (VIR_RESIZE_N(*params, max, n, 1) < 0) -goto error; -*maxparams = max; +/** + * virTypedParamsPackStrings: + * @params: array of typed parameters + * @nparams: number of parameters in the @params array + * @maxparams: maximum number of parameters that can be stored in @params + * array without allocating more memory + * @name: name of the parameter to store values to + * @values: the values to store into the new parameters + * + * Packs NULL-terminated list of strings @values into @params under the + * key @name. + * + * Returns 0 on success, -1 on error. + */ +int +virTypedParamsPackStrings(virTypedParameterPtr *params, + int *nparams, + int *maxparams, + const char *name, + const char **values) +{ +size_t i; +int rv = -1; -if (VIR_STRDUP(str, value) < 0) -goto error; +if (!values) +return 0; -if (virTypedParameterAssign(*params + n, name, -VIR_TYPED_PARAM_STRING, str) < 0) { -VIR_FREE(str); -goto error; +for (i = 0; values[i]; i++) { +if ((rv = virTypedParamsAddStringFull(params, nparams, maxparams, + name, values[i], 0)) < 0) +break; } -*nparams += 1; -return 0; - - error: -virDispatchError(NULL); -return -1; +return rv; } diff --git a/tests/virtypedparamtest.c b/tests/virtypedparamtest.c index 287d3f1..037d5d1 100644 --- a/tests/virtypedparamtest.c +++ b/tests/virtypedparamtest.c @@ -139,6
[libvirt] [PATCH 0/5] Selective block device migration implementation
The patchset represented in the mail thread implements the selective block device migration for the QEMU driver. This closes the referenced bug [1]. First the supplementary API implemented allowing for multiple key-values pair in the virTypedParameter arrays. This is used to pass the list of block devices to migrate in the following patches. Unit tests for this are added as well. This is the subject of the first three patches. Fourth patch is adding the necessary parameter `migrate_disks' and passes it through the QEMU driver call stack. The introduced `qemuMigrateBlockDevice' function is then checks that the disk is to be migrated because it is in the list. If there is no list specified then the legacy check is used: the device is not shared, not readonly and has a source. Fifth and the last patch is adding the necessary code to the `virsh' utility making it able for user to specify a comma-separated list of the block device names that are to be migrated. The implemented Python bindings patch is to be presented. [1] https://bugzilla.redhat.com/show_bug.cgi?id=1203032 Pavel Boldin (5): util: multi-value virTypedParameter util: virTypedParamsPick* multikey API util: add virTypedParamsPackStrings qemu: migration: selective block device migration virsh: selective block device migration include/libvirt/libvirt-domain.h | 9 ++ include/libvirt/libvirt-host.h | 24 src/libvirt_public.syms | 3 + src/qemu/qemu_driver.c | 63 ++--- src/qemu/qemu_migration.c| 197 -- src/qemu/qemu_migration.h| 23 ++-- src/util/virtypedparam.c | 290 +++ tests/Makefile.am| 6 + tests/virtypedparamtest.c| 289 ++ tools/virsh-domain.c | 44 ++ 10 files changed, 788 insertions(+), 160 deletions(-) create mode 100644 tests/virtypedparamtest.c -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/5] qemu: migration: selective block device migration
Implement a `migrate_disks' parameters for the QEMU driver. This multi- value parameter can be used to explicitly specify what block devices are to be migrated using the NBD server. Tunnelled migration using NBD is to be done. Signed-off-by: Pavel Boldin --- include/libvirt/libvirt-domain.h | 9 ++ src/qemu/qemu_driver.c | 63 + src/qemu/qemu_migration.c| 198 --- src/qemu/qemu_migration.h| 23 +++-- 4 files changed, 192 insertions(+), 101 deletions(-) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 0f465b9..6b48923 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -748,6 +748,15 @@ typedef enum { */ # define VIR_MIGRATE_PARAM_LISTEN_ADDRESS"listen_address" +/** + * VIR_MIGRATE_PARAM_MIGRATE_DISKS: + * + * virDomainMigrate* params multiple field: The multiple values that list + * the block devices to be migrated. At the moment this is only supported + * by the QEMU driver but not for the tunnelled migration. + */ +# define VIR_MIGRATE_PARAM_MIGRATE_DISKS"migrate_disks" + /* Domain migration. */ virDomainPtr virDomainMigrate (virDomainPtr domain, virConnectPtr dconn, unsigned long flags, const char *dname, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3695b26..2f53df6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12472,7 +12472,7 @@ qemuDomainMigratePrepare2(virConnectPtr dconn, ret = qemuMigrationPrepareDirect(driver, dconn, NULL, 0, NULL, NULL, /* No cookies */ uri_in, uri_out, - &def, origname, NULL, flags); + &def, origname, NULL, flags, NULL); cleanup: VIR_FREE(origname); @@ -12525,7 +12525,7 @@ qemuDomainMigratePerform(virDomainPtr dom, * Consume any cookie we were able to decode though */ ret = qemuMigrationPerform(driver, dom->conn, vm, - NULL, dconnuri, uri, NULL, NULL, + NULL, dconnuri, uri, NULL, NULL, NULL, cookie, cookielen, NULL, NULL, /* No output cookies in v2 */ flags, dname, resource, false); @@ -12602,7 +12602,7 @@ qemuDomainMigrateBegin3(virDomainPtr domain, } return qemuMigrationBegin(domain->conn, vm, xmlin, dname, - cookieout, cookieoutlen, flags); + cookieout, cookieoutlen, flags, NULL); } static char * @@ -12615,11 +12615,13 @@ qemuDomainMigrateBegin3Params(virDomainPtr domain, { const char *xmlin = NULL; const char *dname = NULL; +const char **migrate_disks = NULL; +char *ret = NULL; virDomainObjPtr vm; virCheckFlags(QEMU_MIGRATION_FLAGS, NULL); if (virTypedParamsValidate(params, nparams, QEMU_MIGRATION_PARAMETERS) < 0) -return NULL; +goto cleanup; if (virTypedParamsGetString(params, nparams, VIR_MIGRATE_PARAM_DEST_XML, @@ -12627,18 +12629,25 @@ qemuDomainMigrateBegin3Params(virDomainPtr domain, virTypedParamsGetString(params, nparams, VIR_MIGRATE_PARAM_DEST_NAME, &dname) < 0) -return NULL; +goto cleanup; + +migrate_disks = virTypedParamsPickStrings(params, nparams, + VIR_MIGRATE_PARAM_MIGRATE_DISKS); if (!(vm = qemuDomObjFromDomain(domain))) -return NULL; +goto cleanup; if (virDomainMigrateBegin3ParamsEnsureACL(domain->conn, vm->def) < 0) { virDomainObjEndAPI(&vm); -return NULL; +goto cleanup; } -return qemuMigrationBegin(domain->conn, vm, xmlin, dname, - cookieout, cookieoutlen, flags); +ret = qemuMigrationBegin(domain->conn, vm, xmlin, dname, + cookieout, cookieoutlen, flags, migrate_disks); + + cleanup: +VIR_FREE(migrate_disks); +return ret; } @@ -12682,7 +12691,8 @@ qemuDomainMigratePrepare3(virConnectPtr dconn, cookiein, cookieinlen, cookieout, cookieoutlen, uri_in, uri_out, - &def, origname, NULL, flags); + &def, origname, NULL, flags, + NULL); cleanup: VIR_FREE(origname); @@ -12708,6 +12718,7 @@ qemuDomainMigratePrepare3Params(virConnectPtr dconn, const char *dname = NULL; const char *uri_in = NULL; const char *listenAddress = cfg->migra
[libvirt] [PATCH 2/5] util: virTypedParamsPick* multikey API
Add multikey APIs for virTypedParams*: * virTypedParamsPick that returns all the parameters with the specified name and type. * virTypedParamsPickStrings that returns a NULL-terminated `const char**' list with all the values for specified name and string type. Signed-off-by: Pavel Boldin --- include/libvirt/libvirt-host.h | 10 + src/libvirt_public.syms| 6 +++ src/util/virtypedparam.c | 90 ++ tests/virtypedparamtest.c | 87 4 files changed, 193 insertions(+) diff --git a/include/libvirt/libvirt-host.h b/include/libvirt/libvirt-host.h index a3e8b88..afa730f 100644 --- a/include/libvirt/libvirt-host.h +++ b/include/libvirt/libvirt-host.h @@ -256,6 +256,12 @@ virTypedParameterPtr virTypedParamsGet (virTypedParameterPtr params, int nparams, const char *name); +virTypedParameterPtr* +virTypedParamsPick (virTypedParameterPtr params, + int nparams, + const char *name, + int type, + size_t *picked); int virTypedParamsGetInt(virTypedParameterPtr params, int nparams, @@ -291,6 +297,10 @@ virTypedParamsGetString (virTypedParameterPtr params, int nparams, const char *name, const char **value); +const char ** +virTypedParamsPickStrings(virTypedParameterPtr params, + int nparams, + const char *name); int virTypedParamsAddInt(virTypedParameterPtr *params, int *nparams, diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index ef3d2f0..d886967 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -710,4 +710,10 @@ LIBVIRT_1.2.15 { virDomainDelIOThread; } LIBVIRT_1.2.14; +LIBVIRT_1.2.16 { +global: +virTypedParamsPick; +virTypedParamsPickStrings; +} LIBVIRT_1.2.15; + # define new API here using predicted next version number diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c index bd47609..643d10e 100644 --- a/src/util/virtypedparam.c +++ b/src/util/virtypedparam.c @@ -480,6 +480,56 @@ virTypedParamsGet(virTypedParameterPtr params, } +/** + * virTypedParamsPick: + * @params: array of typed parameters + * @nparams: number of parameters in the @params array + * @name: name of the parameter to find + * @type: type of the parameter to find + * @picked: pointer to a size_t with amount of picked + * + * + * Finds typed parameters called @name. + * + * Returns pointers to the parameters or NULL if there are none in @params. + * This function does not raise an error, even when returning NULL. + * Callee should call VIR_FREE on the returned array. + */ +virTypedParameterPtr* +virTypedParamsPick(virTypedParameterPtr params, + int nparams, + const char *name, + int type, + size_t *picked) +{ +size_t i, max = 0; +virTypedParameterPtr *values = NULL; + +*picked = 0; + +if (!params || !name) +return NULL; + +for (i = 0; i < nparams; i++) { +if (params[i].type == type && STREQ(params[i].field, name)) { +if (VIR_RESIZE_N(values, max, *picked, 1) < 0) +goto error; + +values[*picked] = ¶ms[i]; + +*picked += 1; +} +} + +return values; + + error: +*picked = 0; +VIR_FREE(values); +return NULL; +} + + #define VIR_TYPED_PARAM_CHECK_TYPE(check_type) \ do { if (param->type != check_type) { \ virReportError(VIR_ERR_INVALID_ARG, \ @@ -747,6 +797,46 @@ virTypedParamsGetString(virTypedParameterPtr params, } +/** + * virTypedParamsPickStrings: + * @params: array of typed parameters + * @nparams: number of parameters in the @params array + * @name: name of the parameter to find + * + * + * Finds typed parameters called @name. + * + * Returns pointers to the parameters or NULL if there are none in @params. + * This function does not raise an error, even when returning NULL. + * Callee should call VIR_FREE on the returned array. + */ +const char ** +virTypedParamsPickStrings(virTypedParameterPtr params, + int nparams, const char *name) +{ +const char **values = NULL; +size_t i, picked; +virTypedParameterPtr *picked_params; + +picked_params = virTypedParamsPick(params, nparams, + name, VIR_TYPED_PARAM_STRING, + &picked); + +if (picked_params == NULL) +return NULL; + +if (VIR_ALLOC_N(values, picked + 1) < 0) +goto cleanup; + +for (i = 0; i &
[libvirt] [PATCH 5/5] virsh: selective block device migration
Add `virsh migrate' option `--migratedisks' that allows CLI user to explicitly specify block devices to migrate. Signed-off-by: Pavel Boldin --- tools/virsh-domain.c | 43 +++ 1 file changed, 43 insertions(+) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 4b627e1..4f43a25 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -9793,6 +9793,10 @@ static const vshCmdOptDef opts_migrate[] = { .type = VSH_OT_STRING, .help = N_("filename containing updated XML for the target") }, +{.name = "migratedisks", + .type = VSH_OT_STRING, + .help = N_("comma separated list of disks to be migrated") +}, {.name = NULL} }; @@ -9852,6 +9856,45 @@ doMigrate(void *opaque) VIR_MIGRATE_PARAM_DEST_NAME, opt) < 0) goto save_error; +if (vshCommandOptStringReq(ctl, cmd, "migratedisks", &opt) < 0) +goto out; +if (opt) { +const char **val = NULL; +char *tok, *saveptr = NULL, *opt_copy = NULL, *optp; +size_t max = 0, n = 0; + +if (VIR_STRDUP(opt_copy, opt) < 0) +goto save_error; + +optp = opt_copy; +do { +tok = strtok_r(optp, ",", &saveptr); +optp = NULL; + +if (VIR_RESIZE_N(val, max, n, 1) < 0) { +VIR_FREE(opt_copy); +VIR_FREE(val); +goto save_error; +} + +val[n] = tok; +n++; +} while (tok != NULL); + +if (virTypedParamsPackStrings(¶ms, + &nparams, + &maxparams, + VIR_MIGRATE_PARAM_MIGRATE_DISKS, + val) < 0) { +VIR_FREE(opt_copy); +VIR_FREE(val); +goto save_error; +} + +VIR_FREE(opt_copy); +VIR_FREE(val); +} + if (vshCommandOptStringReq(ctl, cmd, "xml", &opt) < 0) goto out; if (opt) { -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [BUG?] EAGAIN not triggering error and 'events' gets cleared
Thanks Peter, It looks good but I did no deep introspection of the code. Should I rebase my patch [1] on that? I can test your code as well then. [1] https://github.com/paboldin/libvirt/commits/master Pavel On Tue, Apr 28, 2015 at 4:42 PM, Peter Krempa wrote: > On Tue, Apr 28, 2015 at 11:24:37 +0200, Michal Privoznik wrote: > > On 28.04.2015 11:06, Pavel Boldin wrote: > > > Well, actually that seems to be quite a different bug in there. > > > > > > I will start a new thread. > > > > > > In short: migration seems to be broken by commit > > > 1a92c719101e5bfa6fe2b78006ad04c7f075ea28. This is because introduced > job > > > _MODIFY waits while MIGRATION_OUT is finished to change `mirrorState' > > > variable. This deadlocks the libvirt. > > > > Yep, this is known bug. I've told Peter already like two weeks ago. He > > promised to fix it. It would be nice if we can get the fix into the > release. > > There are already patches for the issue: > > http://www.redhat.com/archives/libvir-list/2015-April/msg00724.html > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [BUG?] EAGAIN not triggering error and 'events' gets cleared
Well, actually that seems to be quite a different bug in there. I will start a new thread. In short: migration seems to be broken by commit 1a92c719101e5bfa6fe2b78006ad04c7f075ea28. This is because introduced job _MODIFY waits while MIGRATION_OUT is finished to change `mirrorState' variable. This deadlocks the libvirt. Pavel On Tue, Apr 28, 2015 at 6:29 AM, Pavel Boldin wrote: > Dear Libvirt Developers, > > There seems to be a bug or at least a bad behavior in > `src/qemu/qemu_monitor.c' lines 683-689 function `qemuMonitorIO': > > if (qemuMonitorIOWrite(mon) < 0) { > error = true; > if (errno == ECONNRESET) > hangup = true; > } > events &= ~VIR_EVENT_HANDLE_WRITABLE; > > The `qemuMonitorIOWrite' is returning 0 in case 'write' returns EAGAIN > thus 'events' is always cleared of `VIR_EVENT_HANDLE_WRITABLE' even in case > no message have been sent indeed. > > The question is: who is responsible for handling this? It seems like > 'errno' is getting overwritten inside all of qemuMonitorJSON* functions so > the caller can't rely on it and it needs to be fixed inside the > `qemuMonitorIO'. > > Pavel > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [BUG?] EAGAIN not triggering error and 'events' gets cleared
Dear Libvirt Developers, There seems to be a bug or at least a bad behavior in `src/qemu/qemu_monitor.c' lines 683-689 function `qemuMonitorIO': if (qemuMonitorIOWrite(mon) < 0) { error = true; if (errno == ECONNRESET) hangup = true; } events &= ~VIR_EVENT_HANDLE_WRITABLE; The `qemuMonitorIOWrite' is returning 0 in case 'write' returns EAGAIN thus 'events' is always cleared of `VIR_EVENT_HANDLE_WRITABLE' even in case no message have been sent indeed. The question is: who is responsible for handling this? It seems like 'errno' is getting overwritten inside all of qemuMonitorJSON* functions so the caller can't rely on it and it needs to be fixed inside the `qemuMonitorIO'. Pavel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Selective block device migration implementation
> That's why I've chosen to work purely with disk target at NBD level. We > have strong rules what characters can occur there. Moreover, it's fairly > easy to derive qemu disk ID from the target. Oh, and we require targets > to be unique throughout the domain. So I think it's the best option for > the extension you're planning. > Bad news is NBD is not enabled for tunnelled migration: (src/qemu/qemu_migration.c, line 2635). /* TODO support NBD for TUNNELLED migration */ if (flags & VIR_MIGRATE_TUNNELLED) { VIR_WARN("NBD in tunnelled migration is currently not supported"); } else { cookieFlags |= QEMU_MIGRATION_COOKIE_NBD; priv->nbdPort = 0; } So, we will need to implement this first for my code path, since all the migrations started by the Nova a tunnelled. > Hm.. what about this, introduce just this new function: > > virTypedParamsGetArrayForKey(.., const char *key, ...) > Since we are using libvirt-python in nova I will have to change the way python bindings convert input `dict' to a TypedParams. For this, the `virPyDictToTypedParams' would have to be changed. One cannot pass a multiple values with the same key in Python so `virPyDictToTypedParams' will have to translate dict(migrate_disk=[1,2,3]) into TypedParams {("migrate_disk", 1), ("migrate_disk", 2), ("migrate_disk", 3)}. Pavel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Selective block device migration implementation
On Wed, Apr 15, 2015 at 6:43 PM, Michal Privoznik wrote: > On 15.04.2015 16:38, Pavel Boldin wrote: > > Michal, > > > > On Wed, Apr 15, 2015 at 10:54 AM, Michal Privoznik > > wrote: > > > >> On 26.03.2015 15:48, Pavel Boldin wrote: > >>> Dear Libvirt Developers, > >>> > >>> > >>> I'm working to implement feature request [1]. The feature request > >> proposes > >>> to enhance `libvirt' code so the API caller can specify which block > >> devices > >>> are to be migrated using e.g. parameters in the > `virDomainMigrateToURI3' > >>> call. > >> > >> Correct. My idea is to add pairs of typed parameters to the > >> virDomainMigrate3() and virDomainMigrateToURI3() API. These are the only > >> migrate APIs that accept virTypedParameter. The rest of migrate APIs are > >> just too old and not extensible, so they must continue working as they > >> are now. However, those two APIs - they can be passed new pairs, e.g. > >> ("migrate_disk", "vda"). Now, mgmt applications will want to specify > >> more than one disk to migrate, obviously. We have two options: > >> > >> 1) make "migrate_disk" accept stringyfied array of disk targets, e.g. > >> "migrate_disk": "vda,vdb,sda". This is not so pleasant to work with from > >> client applications. Constructing the string would be awful. > >> > > My concern was that some drivers may allow commas inside the drive names. > > So this is obviously wrong thing to do then. > > That's why I've chosen to work purely with disk target at NBD level. We > have strong rules what characters can occur there. Moreover, it's fairly > easy to derive qemu disk ID from the target. Oh, and we require targets > to be unique throughout the domain. So I think it's the best option for > the extension you're planning. > As far as I remember, libvirt-1.2.2 does not support tunneled migration through NBD. > > > > > > >> > >> 2) make virTypedParam* APIs accept more than one values to a key, e.g. > >> {("migrate_disk", "vda"), ("migrate_disk", "vdb"), ("migrate_disk", > >> "sda")}. Currently, this is supported on RPC, but some virTypedParam* > >> APIs are not really prepared for this. If I had to name some from the > >> top of my head: virTypedParamsGet(). > >> > > As far as I see there is a line 420 in src/util/virtypedparam.c: > > /* The following APIs are public and their signature may never change. */ > > Of course. They would still return the first value found. But that's not > a problem. The paragraph is more like an intro than a suggestion to > change them. > > > > > So, the functions need to be implemented anew. > > Some of them. Correct. > > > > > > > > >> > >> I view virTyped* as an associative array. So 2) is practically enabling > >> multi value associative array. So I guess the first step would be to > >> introduce new API (maybe set of APIs?) that are aware of this. > >> > > I guess the first function to implement would be virTypedParamsGetNth() > > that is able to retrieve nth parameter with that name. > > While this is easy to implement there are details remain: > > > >1. Should we implement a new set of virTypedParamsGetNth for > each > >of the TYPE or should we start with just the 'String'? > >2. Since virTypedParamsGet is just a virTypedParamsGetNth > >with N=0 should we make old functions just call new ones with N=0? > > > > Hm.. what about this, introduce just this new function: > > virTypedParamsGetArrayForKey(.., const char *key, ...) > > which will iterate through an array of typed params, producing a new > array which will, however, contain only the selected key. For instance, > if called over > > {(key1, val1), (key1, val2), (key2, val3), (key4, val4)} > > with key==key1 it would create array > > {(key1, val1), (key1, val2)} > > with key==key2 it would create array > > {(key2, val3)} > > And so on. For selecting Nth item in the array, we can just use basic C > array selector []. Oh, I don't like the function name much btw, but it > serves the example sake here. > Well, this approach allows us to return virTypedParamPtr array of any type. So this is more general and the latter can be implemented as a call to this one. > > The other option would be to have a different function: >
Re: [libvirt] Selective block device migration implementation
Michal, On Wed, Apr 15, 2015 at 10:54 AM, Michal Privoznik wrote: > On 26.03.2015 15:48, Pavel Boldin wrote: > > Dear Libvirt Developers, > > > > > > I'm working to implement feature request [1]. The feature request > proposes > > to enhance `libvirt' code so the API caller can specify which block > devices > > are to be migrated using e.g. parameters in the `virDomainMigrateToURI3' > > call. > > Correct. My idea is to add pairs of typed parameters to the > virDomainMigrate3() and virDomainMigrateToURI3() API. These are the only > migrate APIs that accept virTypedParameter. The rest of migrate APIs are > just too old and not extensible, so they must continue working as they > are now. However, those two APIs - they can be passed new pairs, e.g. > ("migrate_disk", "vda"). Now, mgmt applications will want to specify > more than one disk to migrate, obviously. We have two options: > > 1) make "migrate_disk" accept stringyfied array of disk targets, e.g. > "migrate_disk": "vda,vdb,sda". This is not so pleasant to work with from > client applications. Constructing the string would be awful. > My concern was that some drivers may allow commas inside the drive names. So this is obviously wrong thing to do then. > > 2) make virTypedParam* APIs accept more than one values to a key, e.g. > {("migrate_disk", "vda"), ("migrate_disk", "vdb"), ("migrate_disk", > "sda")}. Currently, this is supported on RPC, but some virTypedParam* > APIs are not really prepared for this. If I had to name some from the > top of my head: virTypedParamsGet(). > As far as I see there is a line 420 in src/util/virtypedparam.c: /* The following APIs are public and their signature may never change. */ So, the functions need to be implemented anew. > > I view virTyped* as an associative array. So 2) is practically enabling > multi value associative array. So I guess the first step would be to > introduce new API (maybe set of APIs?) that are aware of this. > I guess the first function to implement would be virTypedParamsGetNth() that is able to retrieve nth parameter with that name. While this is easy to implement there are details remain: 1. Should we implement a new set of virTypedParamsGetNth for each of the TYPE or should we start with just the 'String'? 2. Since virTypedParamsGet is just a virTypedParamsGetNth with N=0 should we make old functions just call new ones with N=0? > > Then, virDomainMigrate*3() can just use this in the following way: > > a) when no "migrate_disk" is specified, the current behaviour is preserved > > b) when there are some disk selected for storage migration, since at > this point we have virTyped* APIs to work with multivalue, we can get > the array of disk targets to migrate and instruct qemu to just migrate > them. > > Now, qemu has split storage and internal state migration into two > streams. The first one is called NBD and libvirt selectively choses > disks to migrate. For the other stream you don't have to care about. > Look at qemuMigrationDriveMirror() and you'll see how libvirt instructs > qemu to selectively migrate only some disks. The "migrate_disk" array > would need to be propagated down here then. > My concern is I would, most likely, have to backport these to our versions. Pavel > Hopefully I haven't forgot nothing. > > Michal > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Selective block device migration implementation
Dear Libvirt Developers, I'm working to implement feature request [1]. The feature request proposes to enhance `libvirt' code so the API caller can specify which block devices are to be migrated using e.g. parameters in the `virDomainMigrateToURI3' call. There is the following issues: 1. It is obvious to identify devices to be migrated using the `device_name'. However, these need to be serialized in either 1) a comma-separated string or in 2) a set of values named like `blockdevice%d' with `blockdevice' value holding amount of block devices to migrate. What is the desired approach here? Can block device name contain commas or should I neglect this possibility? 2. `libvirt' behavior in block devices migration was copied from the `QEMU' implementation that ignores read-only devices as well. So, `libvirt' code will need to pass this list to `QEMU' `migrate' QMP command and this argument should be implemented in the `QEMU'. Is there any implementation advices from your side? Pavel [1] https://bugzilla.redhat.com/show_bug.cgi?id=1203032 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list