Hi Michal,
On 8/14/14, 2:58 , "Michal Privoznik" <mprivozn@xxxxxxxxxx> wrote:
>On 13.08.2014 17:42, Tomoki Sekiyama wrote:
>> This patch gives users a nicer error message when the QEMU guest agent
>>is
>> not new enough to support 'guest-fsfreeze-freeze-list' command, which is
>> used by qemuDomainFSFreeze() to freeze specified filesystems only.
>>
>> Before this patch, it was depending on the agent to complain about
>>unknown
>> command:
>> # virsh domfsfreeze domain --mountpoint /mnt/point
>> error: Unable to freeze filesystems
>> error: internal error: unable to execute QEMU agent command 'guest-
>> fsfreeze-freeze-list': The command guest-fsfreeze-freeze-list has
>>not been
>> found
>>
>> After:
>> # virsh domfsfreeze domain --mountpoint /mnt/point
>> error: Unable to freeze filesystems
>> error: argument unsupported: this version of guest agent doesn't
>>support
>> specifying mountpoints
>>
>> Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama@xxxxxxx>
>> ---
<snip>
>It's not that I'm against this patch in general. I'm just against it now
>:) I mean, with current implementation libvirt doesn't know what qemu-ga
>is it talking to. The guest agent can come and go, without libvirt
>noticing anything [1]. So in this specific case, guest-info can be
>executed by say newer qemu-ga which does support
>guest-fsfreeze-freeze-list and thus direct libvirt to use it. But then
>the freeze command is executed by downgraded qemu-ga so user will still
>see the ugly error message.
>
>At qemu monitor level it makes sense to query supported commands and
>capabilities because monitor disappears if and only if the qemu process
>dies. The assumptions is however not true with qemu-ga.
>
>1: The long reasoning above is not entirely true. There were attempts to
>improve this. Previously, qemu was (and from libvirt's POV still is) a
>middle man, so it's impossible from outside to tell if somebody's
>listening on the guest side of a virtio channel. But things have changed
>since then and qemu-2.1 should emit an monitor event whenever qemu-ga
>(dis-)connects. And libvirt could then react to connect event by
>executing guest-info. However doing it without events doesn't make much
>sense to me. Sorry.
I understood. This requires to distinguish version of qemu-ga older than
2.1 too, so using guest-info (outside the connect event) for capabilities
is impossible.
>What we can do is, something like this (untested, just idea):
>
>diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
>index a10954a..8bc04e5 100644
>--- a/src/qemu/qemu_agent.c
>+++ b/src/qemu/qemu_agent.c
>@@ -1346,8 +1346,19 @@ int qemuAgentFSFreeze(qemuAgentPtr mon, const char
>**mountpoints,
> return -1;
>
> if (qemuAgentCommand(mon, cmd, &reply, true,
>- VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0)
>+ VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0) {
>+ if (mountpoints && nmountpoints &&
>+ virJSONValueObjectHasKey(reply, "error")) {
>+ virJSONValuePtr error = virJSONValueObjectGet(reply,
>"error");
>+ const char *klass = virJSONValueObjectGetString(error,
>"class");
>+ if (STREQ(klass, "CommandNotFound")) {
>+ virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
>+ _("this version of guest agent doesn't
>support "
>+ "specifying mountpoints"));
>+ }
>+ }
> goto cleanup;
>+ }
>
> if (virJSONValueObjectGetNumberInt(reply, "return", &ret) < 0) {
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
I¹ve tested this and confirmed that it works as expected.
I¹m okey with this patch instead of mine.
>Although I'm not sure it's worth putting so much effort in to just
>printing nicer error message.
And now, even I think it is reasonable to rely on qemu-ga¹s error message,
until we have capability detection using the agent (dis-)connect events.
Thanks,
Tomoki Sekiyama
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list