>On Wed, Mar 05, 2014 at 02:53:10PM -0500, Tomoki Sekiyama wrote:
>> 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 */
>> +
v> + bool quiesced; /* true if the domain filesystems are quiesced */
>> };
>>
>> typedef enum {
>
> You will also want to update qemuDomainObjPrivateXMLFormat and
> qemuDomainObjPrivateXMLParse so that this new 'quiesced' attribute
> is preserved in the live XML state across libvirtd restarts.
OK, I will update them too.
>> 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;
>> }
>
> And in both these methods you want to call virDomainSaveStatus after changing
> the priv->quiesced flag, to save the status XML to disk
OK. Thanks for pointing this out.
>>
>> /* 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;
>> }
>
> I'm not sure I understand why you're changing this ? Are you trying to
> deal with the case where the guest is already frozen, but someone has
> still supplied the VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE flag ?
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.
> I'd probably argue that such usage is a bug we should report to the
> caller as an error.
Thanks,
Tomoki Sekiyama
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list