On Mon, Apr 16, 2012 at 05:09:59PM +0200, Michal Privoznik wrote:
> Currently, we have 3 boolean arguments we have to pass
> to qemuProcessStart(). As libvirt grows it is harder and harder
> to remember them and their position. Therefore we should
> switch to flags instead.
> ---
> diff to v1:
> -fix a test for START_PAUSED flag
>
> src/qemu/qemu_driver.c | 45 +++++++++++++++++++++++++++++----------------
> src/qemu/qemu_migration.c | 7 ++++---
> src/qemu/qemu_process.c | 21 +++++++++++++--------
> src/qemu/qemu_process.h | 12 ++++++++----
> 4 files changed, 54 insertions(+), 31 deletions(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 65ed290..436ef37 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -1351,10 +1351,16 @@ static virDomainPtr qemudDomainCreate(virConnectPtr conn, const char *xml,
> virDomainPtr dom = NULL;
> virDomainEventPtr event = NULL;
> virDomainEventPtr event2 = NULL;
> + unsigned int start_flags = VIR_QEMU_PROCESS_START_COLD;
>
> virCheckFlags(VIR_DOMAIN_START_PAUSED |
> VIR_DOMAIN_START_AUTODESTROY, NULL);
>
> + if (flags & VIR_DOMAIN_START_PAUSED)
> + start_flags |= VIR_QEMU_PROCESS_START_PAUSED;
> + if (flags & VIR_DOMAIN_START_AUTODESTROY)
> + start_flags |= VIR_QEMU_PROCESS_START_AUTODESROY;
> +
> qemuDriverLock(driver);
> if (!(def = virDomainDefParseString(driver->caps, xml,
> QEMU_EXPECTED_VIRT_TYPES,
> @@ -1383,10 +1389,9 @@ static virDomainPtr qemudDomainCreate(virConnectPtr conn, const char *xml,
> if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) < 0)
> goto cleanup; /* XXXX free the 'vm' we created ? */
>
> - if (qemuProcessStart(conn, driver, vm, NULL, true,
> - (flags & VIR_DOMAIN_START_PAUSED) != 0,
> - (flags & VIR_DOMAIN_START_AUTODESTROY) != 0,
> - -1, NULL, NULL, VIR_NETDEV_VPORT_PROFILE_OP_CREATE) < 0) {
> + if (qemuProcessStart(conn, driver, vm, NULL, -1, NULL, NULL,
> + VIR_NETDEV_VPORT_PROFILE_OP_CREATE,
> + start_flags) < 0) {
> virDomainAuditStart(vm, "booted", false);
> if (qemuDomainObjEndJob(driver, vm) > 0)
> qemuDomainRemoveInactive(driver, vm);
> @@ -4138,9 +4143,9 @@ qemuDomainSaveImageStartVM(virConnectPtr conn,
> }
>
> /* Set the migration source and start it up. */
> - ret = qemuProcessStart(conn, driver, vm, "stdio", false, true,
> - false, *fd, path, NULL,
> - VIR_NETDEV_VPORT_PROFILE_OP_RESTORE);
> + ret = qemuProcessStart(conn, driver, vm, "stdio", *fd, path, NULL,
> + VIR_NETDEV_VPORT_PROFILE_OP_RESTORE,
> + VIR_QEMU_PROCESS_START_PAUSED);
>
> if (intermediatefd != -1) {
> if (ret < 0) {
> @@ -4710,6 +4715,10 @@ qemuDomainObjStart(virConnectPtr conn,
> bool autodestroy = (flags & VIR_DOMAIN_START_AUTODESTROY) != 0;
> bool bypass_cache = (flags & VIR_DOMAIN_START_BYPASS_CACHE) != 0;
> bool force_boot = (flags & VIR_DOMAIN_START_FORCE_BOOT) != 0;
> + unsigned int start_flags = VIR_QEMU_PROCESS_START_COLD;
> +
> + start_flags |= start_paused ? VIR_QEMU_PROCESS_START_PAUSED : 0;
> + start_flags |= autodestroy ? VIR_QEMU_PROCESS_START_AUTODESROY : 0;
>
> /*
> * If there is a managed saved state restore it instead of starting
> @@ -4741,9 +4750,8 @@ qemuDomainObjStart(virConnectPtr conn,
> }
> }
>
> - ret = qemuProcessStart(conn, driver, vm, NULL, true, start_paused,
> - autodestroy, -1, NULL, NULL,
> - VIR_NETDEV_VPORT_PROFILE_OP_CREATE);
> + ret = qemuProcessStart(conn, driver, vm, NULL, -1, NULL, NULL,
> + VIR_NETDEV_VPORT_PROFILE_OP_CREATE, start_flags);
> virDomainAuditStart(vm, "booted", ret >= 0);
> if (ret >= 0) {
> virDomainEventPtr event =
> @@ -11027,9 +11035,10 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
> if (config)
> virDomainObjAssignDef(vm, config, false);
>
> - rc = qemuProcessStart(snapshot->domain->conn, driver, vm, NULL,
> - false, true, false, -1, NULL, snap,
> - VIR_NETDEV_VPORT_PROFILE_OP_CREATE);
> + rc = qemuProcessStart(snapshot->domain->conn,
> + driver, vm, NULL, -1, NULL, snap,
> + VIR_NETDEV_VPORT_PROFILE_OP_CREATE,
> + VIR_QEMU_PROCESS_START_PAUSED);
> virDomainAuditStart(vm, "from-snapshot", rc >= 0);
> detail = VIR_DOMAIN_EVENT_STARTED_FROM_SNAPSHOT;
> event = virDomainEventNewFromObj(vm,
> @@ -11114,12 +11123,16 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
> VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED)) {
> /* Flush first event, now do transition 2 or 3 */
> bool paused = (flags & VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED) != 0;
> + unsigned int start_flags = 0;
> +
> + start_flags |= paused ? VIR_QEMU_PROCESS_START_PAUSED : 0;
>
> if (event)
> qemuDomainEventQueue(driver, event);
> - rc = qemuProcessStart(snapshot->domain->conn, driver, vm, NULL,
> - false, paused, false, -1, NULL, NULL,
> - VIR_NETDEV_VPORT_PROFILE_OP_CREATE);
> + rc = qemuProcessStart(snapshot->domain->conn,
> + driver, vm, NULL, -1, NULL, NULL,
> + VIR_NETDEV_VPORT_PROFILE_OP_CREATE,
> + start_flags);
> virDomainAuditStart(vm, "from-snapshot", rc >= 0);
> if (rc < 0) {
> if (!vm->persistent) {
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index dc4d616..fc83805 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -1303,9 +1303,10 @@ qemuMigrationPrepareAny(struct qemud_driver *driver,
> /* Start the QEMU daemon, with the same command-line arguments plus
> * -incoming $migrateFrom
> */
> - if (qemuProcessStart(dconn, driver, vm, migrateFrom, false, true,
> - true, dataFD[0], NULL, NULL,
> - VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_START) < 0) {
> + if (qemuProcessStart(dconn, driver, vm, migrateFrom, dataFD[0], NULL, NULL,
> + VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_START,
> + VIR_QEMU_PROCESS_START_PAUSED |
> + VIR_QEMU_PROCESS_START_AUTODESROY) < 0) {
> virDomainAuditStart(vm, "migrated", false);
> /* Note that we don't set an error here because qemuProcessStart
> * should have already done that.
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 692fc32..ecbf039 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -3271,13 +3271,11 @@ int qemuProcessStart(virConnectPtr conn,
> struct qemud_driver *driver,
> virDomainObjPtr vm,
> const char *migrateFrom,
> - bool cold_boot,
> - bool start_paused,
> - bool autodestroy,
> int stdin_fd,
> const char *stdin_path,
> virDomainSnapshotObjPtr snapshot,
> - enum virNetDevVPortProfileOp vmop)
> + enum virNetDevVPortProfileOp vmop,
> + unsigned int flags)
> {
> int ret;
> off_t pos = -1;
> @@ -3290,6 +3288,12 @@ int qemuProcessStart(virConnectPtr conn,
> unsigned long cur_balloon;
> int i;
>
> + /* Okay, these are just internal flags,
> + * but doesn't hurt to check */
> + virCheckFlags(VIR_QEMU_PROCESS_START_COLD |
> + VIR_QEMU_PROCESS_START_PAUSED |
> + VIR_QEMU_PROCESS_START_AUTODESROY, -1);
> +
> hookData.conn = conn;
> hookData.vm = vm;
> hookData.driver = driver;
> @@ -3434,7 +3438,8 @@ int qemuProcessStart(virConnectPtr conn,
> goto cleanup;
>
> VIR_DEBUG("Checking for CDROM and floppy presence");
> - if (qemuDomainCheckDiskPresence(driver, vm, cold_boot) < 0)
> + if (qemuDomainCheckDiskPresence(driver, vm,
> + flags & VIR_QEMU_PROCESS_START_COLD) < 0)
> goto cleanup;
>
> VIR_DEBUG("Setting up domain cgroup (if required)");
> @@ -3633,7 +3638,7 @@ int qemuProcessStart(virConnectPtr conn,
> VIR_DEBUG("Handshake complete, child running");
>
> if (migrateFrom)
> - start_paused = true;
> + flags |= VIR_QEMU_PROCESS_START_PAUSED;
>
> if (ret == -1) /* The VM failed to start; tear filters before taps */
> virDomainConfVMNWFilterTeardown(vm);
> @@ -3708,7 +3713,7 @@ int qemuProcessStart(virConnectPtr conn,
> }
> qemuDomainObjExitMonitorWithDriver(driver, vm);
>
> - if (!start_paused) {
> + if (!(flags & VIR_QEMU_PROCESS_START_PAUSED)) {
> VIR_DEBUG("Starting domain CPUs");
> /* Allow the CPUS to start executing */
> if (qemuProcessStartCPUs(driver, vm, conn,
> @@ -3726,7 +3731,7 @@ int qemuProcessStart(virConnectPtr conn,
> VIR_DOMAIN_PAUSED_USER);
> }
>
> - if (autodestroy &&
> + if (flags & VIR_QEMU_PROCESS_START_AUTODESROY &&
> qemuProcessAutoDestroyAdd(driver, vm, conn) < 0)
> goto cleanup;
>
> diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
> index 5cb5ffc..2d75b005 100644
> --- a/src/qemu/qemu_process.h
> +++ b/src/qemu/qemu_process.h
> @@ -44,17 +44,21 @@ void qemuProcessReconnectAll(virConnectPtr conn, struct qemud_driver *driver);
>
> int qemuProcessAssignPCIAddresses(virDomainDefPtr def);
>
> +typedef enum {
> + VIR_QEMU_PROCESS_START_COLD = 1 << 0,
> + VIR_QEMU_PROCESS_START_PAUSED = 1 << 1,
> + VIR_QEMU_PROCESS_START_AUTODESROY = 1 << 2,
> +} qemuProcessStartFlags;
> +
> int qemuProcessStart(virConnectPtr conn,
> struct qemud_driver *driver,
> virDomainObjPtr vm,
> const char *migrateFrom,
> - bool cold_boot,
> - bool start_paused,
> - bool autodestroy,
> int stdin_fd,
> const char *stdin_path,
> virDomainSnapshotObjPtr snapshot,
> - enum virNetDevVPortProfileOp vmop);
> + enum virNetDevVPortProfileOp vmop,
> + unsigned int flags);
>
> void qemuProcessStop(struct qemud_driver *driver,
> virDomainObjPtr vm,
ACK, that's better now
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list