On Wed, Mar 05, 2014 at 02:53:03PM -0500, Tomoki Sekiyama wrote: I think this is a case where we should invent a new permission name. Add VIR_ACCESS_PERM_DOMAIN_FS_FREEZE to src/access/viraccessperm.h and use it for both of these RPC calls. We should also make the existing VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE flag for snapshot APIs uses this new permission. Regards, 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 :|

Adds an quiesced flag into qemuDomainObjPrivate that tracks whether guest filesystems of the domain is quiesced of not. It also modify error code from qemuDomainSnapshotFSFreeze and qemuDomainSnapshotFSThaw, so that a caller can know whether the command is actually sent to the guest agent. If the error is caused before sending a freeze command, a counterpart thaw command shouldn't be sent to avoid confusing the tracking of quiesced status. --- src/qemu/qemu_domain.h | 2 ++ src/qemu/qemu_driver.c | 41 +++++++++++++++++++++++++++++++---------- 2 files changed, 33 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 0bed50b..8a79a00 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -176,6 +176,8 @@ struct _qemuDomainObjPrivate { char **qemuDevices; /* NULL-terminated list of devices aliases known to QEMU */ bool hookRun; /* true if there was a hook run over this domain */ + + bool quiesced; /* true if the domain filesystems are quiesced */ }; typedef enum { diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 9aad2dc..8109e46 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12040,6 +12040,8 @@ cleanup: } +/* Return -1 if request is not sent to agent due to misconfig, -2 on agent + * failure, and number of frozen filesystems on success. */ static int qemuDomainSnapshotFSFreeze(virDomainObjPtr vm) { qemuDomainObjPrivatePtr priv = vm->privateData; @@ -12056,14 +12058,24 @@ qemuDomainSnapshotFSFreeze(virDomainObjPtr vm) { _("QEMU guest agent is not configured")); return -1; } + if (priv->quiesced) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("domain is already quiesced")); + return -1; + } qemuDomainObjEnterAgent(vm); freezed = qemuAgentFSFreeze(priv->agent); qemuDomainObjExitAgent(vm); - return freezed; + if (freezed >= 0) + priv->quiesced = true; + + return freezed < 0 ? -2 : freezed; } +/* Return -1 if request is not sent to agent due to misconfig, -2 on agent + * failure, and number of thawed filesystems on success. */ static int qemuDomainSnapshotFSThaw(virDomainObjPtr vm, bool report) { @@ -12084,6 +12096,11 @@ qemuDomainSnapshotFSThaw(virDomainObjPtr vm, bool report) _("QEMU guest agent is not configured")); return -1; } + if (!priv->quiesced && report) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("domain is not quiesced")); + return -1; + } qemuDomainObjEnterAgent(vm); if (!report) @@ -12093,8 +12110,11 @@ qemuDomainSnapshotFSThaw(virDomainObjPtr vm, bool report) virSetError(err); qemuDomainObjExitAgent(vm); + if (thawed >= 0) + priv->quiesced = false; + virFreeError(err); - return thawed; + return thawed < 0 ? -2 : thawed; } /* The domain is expected to be locked and inactive. */ @@ -13069,17 +13089,18 @@ qemuDomainSnapshotCreateActiveExternal(virConnectPtr conn, goto cleanup; /* If quiesce was requested, then issue a freeze command, and a - * counterpart thaw command, no matter what. The command will - * fail if the guest is paused or the guest agent is not - * running. */ + * counterpart thaw command when the it is actually sent to agent. + * The command will fail if the guest is paused or the guest agent is not + * running, or is already quiesced. */ if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE) { - if (qemuDomainSnapshotFSFreeze(vm) < 0) { - /* helper reported the error */ - thaw = -1; + int freeze = qemuDomainSnapshotFSFreeze(vm); + if (freeze < 0) { + /* the helper reported the error */ + if (freeze != -1) + thaw = -1; /* the command is sent but agent failed */ goto endjob; - } else { - thaw = 1; } + thaw = 1; } /* We need to track what state the guest is in, since taking the

v> + bool quiesced; /* true if the domain filesystems are quiesced */ OK, I will update them too. OK. Thanks for pointing this out. Right. Without this change, if VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE flag is supplied for a frozen guest: 1. the caller will get 'domain is already quiesced' error 2. the guest is thawed by the error handling (because of 'thaw = 1;' ) 1 is expected behavior (error reporting to the caller), but 2 would be hardly expected from a user's viewpoint. The intention of this change is to avoid 2. Thanks, Tomoki Sekiyama

These are exposed under domfsfreeze command and domfsthaw command. Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama(a)hds.com&gt; --- tools/virsh-domain.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 15 ++++++++ 2 files changed, 107 insertions(+) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 1d3c5f0..329e32e 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -11201,6 +11201,86 @@ cleanup: return ret; } +static const vshCmdInfo info_domfsfreeze[] = { + {.name = "help", + .data = N_("Freeze domain's mounted filesystems.") + }, + {.name = "desc", + .data = N_("Freeze domain's mounted filesystems.") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_domfsfreeze[] = { + {.name = "domain", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_("domain name, id or uuid") + }, + {.name = NULL} +}; +static bool +cmdDomFSFreeze(vshControl *ctl, const vshCmd *cmd) +{ + virDomainPtr dom = NULL; + bool ret = false; + unsigned int flags = 0; + + if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) + return ret; + + if (virDomainFSFreeze(dom, flags) < 0) { + vshError(ctl, _("Unable to freeze filesystems")); + goto cleanup; + } + + ret = true; + +cleanup: + virDomainFree(dom); + return ret; +} + +static const vshCmdInfo info_domfsthaw[] = { + {.name = "help", + .data = N_("Thaw domain's mounted filesystems.") + }, + {.name = "desc", + .data = N_("Thaw domain's mounted filesystems.") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_domfsthaw[] = { + {.name = "domain", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_("domain name, id or uuid") + }, + {.name = NULL} +}; +static bool +cmdDomFSThaw(vshControl *ctl, const vshCmd *cmd) +{ + virDomainPtr dom = NULL; + bool ret = false; + unsigned int flags = 0; + + if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) + return ret; + + if (virDomainFSThaw(dom, flags) < 0) { + vshError(ctl, _("Unable to thaw filesystems")); + goto cleanup; + } + + ret = true; + +cleanup: + virDomainFree(dom); + return ret; +} + const vshCmdDef domManagementCmds[] = { {.name = "attach-device", .handler = cmdAttachDevice, @@ -11348,6 +11428,18 @@ const vshCmdDef domManagementCmds[] = { .info = info_domdisplay, .flags = 0 }, + {.name = "domfsfreeze", + .handler = cmdDomFSFreeze, + .opts = opts_domfsfreeze, + .info = info_domfsfreeze, + .flags = 0 + }, + {.name = "domfsthaw", + .handler = cmdDomFSThaw, + .opts = opts_domfsthaw, + .info = info_domfsthaw, + .flags = 0 + }, {.name = "domfstrim", .handler = cmdDomFSTrim, .opts = opts_domfstrim, diff --git a/tools/virsh.pod b/tools/virsh.pod index cafbb9a..e234bde 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -926,6 +926,21 @@ Output a URI which can be used to connect to the graphical display of the domain via VNC, SPICE or RDP. If I<--include-password> is specified, the SPICE channel password will be included in the URI. +=item B<domfsfreeze> I<domain> + +Freeze all mounted filesystems within a running domain to prepare for +consistent snapshots. + +Note that B<snapshot-create> command has a I<--quiesce> option to freeze +and thaw the filesystems automatically to keep snapshots consistent. +B<domfsfreeze> command is only needed when a user wants to utilize the +native snapshot features of storage devices not supported by libvirt yet. + +=item B<domfsthaw> I<domain> + +Thaw all mounted filesystems within a running domain, which are frozen +by domfsfreeze command. + =item B<domfstrim> I<domain> [I<--minimum> B<bytes>] [I<--mountpoint mountPoint>]