Re: [Qemu-devel] [PATCH v4 06/10] qemu-ga: Add Windows VSS provider to quiesce applications on fsfreeze

From: Laszlo Ersek <lersek@redhat.com>
To: Tomoki Sekiyama <tomoki.sekiyama@hds.com>
Cc: "libaiqing@huawei.com" <libaiqing@huawei.com>,
	"mdroth@linux.vnet.ibm.com" <mdroth@linux.vnet.ibm.com>,
	"stefanha@gmail.com" <stefanha@gmail.com>,
	"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
	"lcapitulino@redhat.com" <lcapitulino@redhat.com>,
	"vrozenfe@redhat.com" <vrozenfe@redhat.com>,
	"pbonzini@redhat.com" <pbonzini@redhat.com>,
	Seiji Aguchi <seiji.aguchi@hds.com>,
	"areis@redhat.com" <areis@redhat.com>
Subject: Re: [Qemu-devel] [PATCH v4 06/10] qemu-ga: Add Windows VSS provider to quiesce applications on fsfreeze
Date: Tue, 25 Jun 2013 21:52:23 +0200	[thread overview]
Message-ID: <51C9F4F7.9090008@redhat.com> (raw)
In-Reply-To: <CDEF3953.2E1E%Tomoki.Sekiyama@hds.com>

On 06/25/13 20:23, Tomoki Sekiyama wrote:
> On 6/25/13 12:03 , "Laszlo Ersek" <lersek@redhat.com> wrote:

>>> +ifeq ($(CONFIG_QGA_VSS),y)
>>> +QEMU_CFLAGS += -DHAS_VSS_SDK
>>
>> Isn't this superfluous? First, I can find no other reference to
>> HAS_VSS_SDK in the series, second, CONFIG_QGA_VSS would be available
>> directly as a macro to source files.
> 
> It is referenced in e.g. qga/commands-win32.c in patch 8/10.

Ah sorry. "ack" (that I sometimes use for recursive grepping, outside of
git clones) didn't return any hits in the directory where I had saved
your patch emails (for git-am), that's why I said "no references in the
series".

Perhaps "ack" ignored the .eml files due to their CRLF line endings...
It did find the references with option "--all".

(Of course I should have noticed something was fishy, since "ack" didn't
find the very occurrence I was questioning! :))

> But as you say, I will omit this, by the second reason.

OK, thanks.

>> Will some VSS service load the provider DLL independently of
>> qemu-ga.exe?
> 
> Yes, VSS provider DLL is registered to VSS on installation of qemu-ga
> service (using .TLB), and loaded into VSS service component when
> snapshot is started.
> 
>> If so, (a) how will they communicate
> 
> Using COM interface.

[...]

>> ... I'll try to continue here. I've peaked forward a little bit, and
>> CQGAVssProvider::CommitSnapshots() seems to be the central method. It
>> kicks the "frozen" event, then blocks until the "thaw" event is kicked
>> back. I wonder how those will translate to QMP communication with
>> libvirt.
> 
> CommitSnapshots() is called by VSS, when the requestor of qemu-ga
> (introduced in next patch) initiates snapshot receiving
> "guest-fsfreeze-freeze" command.
> "frozen" event is used to tell qemu-ga.exe that the fs is frozen,
> then qemu-ga.exe will tell libvirt to "guest-fsfreeze-freeze" is done.
> 
> "thaw" event is kicked by qemnu-ga.exe when it receives
> "guest-fsfreeze-thaw" command. The command is finished when VSS notify
> qeum-ga.exe requestor that VSS snapshot process is completed.

Very interesting. I'm curious about the requestor in the next patch.

Based on the VSS description in MSDN, I thought the requestor would make
some kind of blocking call (like a normal function call) into VSS that
could not return until the full process was done (ie. filesystems were
re-thawed).

This likely isn't the case, as CommitSnapshots() appears to signal
(wake) qemu-ga.exe asynchronously -- the requestor (qemu-ga) and the
provider (the DLL in the VSS component) seem to run in parallel.

The more I think I understand of your design the more I like it.

I'll attempt to continue the review tomorrow.

Thanks!
Laszlo