Re: [pve-devel] [PATCH 2/2] now if the QMP command starts with guest-+ , it will bind dynamicly to the VMID.qga socket. To test the function vmtime is implemented which return the vm UNIX Time.
I thought about IP address and disk usage: guest-network-get-interfaces guest-get-fsinfo So maybe 1 call each minute is enough for this ? But seems guest-get-fsinfo does not return useful data :-/ Yes, I only display mountpoint. (could be useful for hot-unplug protection). Note that both network and fsinfo are not implemented under qga windows currently - Mail original - De: Dietmar Maurer diet...@proxmox.com À: Alexandre DERUMIER aderum...@odiso.com Cc: pve-devel@pve.proxmox.com, Wolfgang Link wolfg...@linksystems.org Envoyé: Mardi 25 Novembre 2014 08:09:39 Objet: RE: [PATCH 2/2] now if the QMP command starts with guest-+ , it will bind dynamicly to the VMID.qga socket. To test the function vmtime is implemented which return the vm UNIX Time. Maybe pvestatd can save if last qga command was successful (guest agent online) what happen here if the guest agent is down ? (for example, when the vm is going to shutdown, stop the guest agent service) We get a timeout, and we save that information so that other processes knows that guest agent is down. BTW, which qga command is used to get status ? I don't find anything related in http://git.qemu.org/?p=qemu.git;a=blob_plain;f=qga/qapi- schema.json;hb=HEAD I thought about IP address and disk usage: guest-network-get-interfaces guest-get-fsinfo But seems guest-get-fsinfo does not return useful data :-/ ___ pve-devel mailing list pve-devel@pve.proxmox.com http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH 2/2] now if the QMP command starts with guest-+ , it will bind dynamicly to the VMID.qga socket. To test the function vmtime is implemented which return the vm UNIX Time.
I thought about IP address and disk usage: guest-network-get-interfaces guest-get-fsinfo So maybe 1 call each minute is enough for this ? yes But seems guest-get-fsinfo does not return useful data :-/ Yes, I only display mountpoint. (could be useful for hot-unplug protection). Note that both network and fsinfo are not implemented under qga windows currently OK, thanks for that info. ___ pve-devel mailing list pve-devel@pve.proxmox.com http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH 2/2] now if the QMP command starts with guest-+ , it will bind dynamicly to the VMID.qga socket.
Signed-off-by: Wolfgang Link wolfg...@linksystems.org --- PVE/QMPClient.pm | 54 + PVE/QemuServer.pm | 16 +--- 2 files changed, 39 insertions(+), 31 deletions(-) diff --git a/PVE/QMPClient.pm b/PVE/QMPClient.pm index 9674d00..b5684b6 100755 --- a/PVE/QMPClient.pm +++ b/PVE/QMPClient.pm @@ -20,7 +20,7 @@ use Data::Dumper; # Note: kvm can onyl handle 1 connection, so we close connections asap sub new { -my ($class, $eventcb, $qga) = @_; +my ($class, $eventcb) = @_; my $mux = new IO::Multiplex; @@ -34,7 +34,6 @@ sub new { }, $class; $self-{eventcb} = $eventcb if $eventcb; -$self-{qga} = $qga if $qga; $mux-set_callback_object($self); @@ -106,9 +105,18 @@ sub cmd { }; my $cmdid_seq = 0; +my $cmdid_seq_qga = 0; + my $next_cmdid = sub { -$cmdid_seq++; -return $$.0.$cmdid_seq; +my ($qga) = @_; + +if($qga){ + $cmdid_seq++; + return $$:$cmdid_seq; +} else { + $cmdid_seq_qga++; + return $$.0.$cmdid_seq_qga; +} }; my $close_connection = sub { @@ -124,9 +132,9 @@ my $close_connection = sub { }; my $open_connection = sub { -my ($self, $vmid, $timeout) = @_; +my ($self, $vmid, $timeout, $qga) = @_; -my $sname = PVE::QemuServer::qmp_socket($vmid, $self-{qga}); +my $sname = PVE::QemuServer::qmp_socket($vmid, $qga); $timeout = 1 if !$timeout; @@ -181,7 +189,8 @@ my $check_queue = sub { eval { my $cmd = $self-{current}-{$vmid} = shift @{$self-{queue}-{$vmid}}; - $cmd-{id} = $next_cmdid(); + + $cmd-{id} = $next_cmdid($cmd-{qga}); my $fd = -1; if ($cmd-{execute} eq 'add-fd' || $cmd-{execute} eq 'getfd') { @@ -191,7 +200,7 @@ my $check_queue = sub { my $qmpcmd = undef; - if($self-{qga}){ + if($self-{current}-{$vmid}-{qga}){ my $qmpcmdid =to_json({ execute = 'guest-sync', @@ -242,11 +251,15 @@ sub queue_execute { # open all necessary connections foreach my $vmid (keys %{$self-{queue}}) { next if !scalar(@{$self-{queue}-{$vmid}}); # no commands for the VM + + if ($self-{queue}-{$vmid}[0]-{execute} =~ /^guest\-+/){ + $self-{queue}-{$vmid}[0]-{qga} = 1; + } eval { - my $fh = $open_connection($self, $vmid, $timeout); + my $fh = $open_connection($self, $vmid, $timeout, $self-{queue}-{$vmid}[0]-{qga}); -if(!$self-{qga}){ +if(!$self-{queue}-{$vmid}[0]-{qga}){ my $cmd = { execute = 'qmp_capabilities', arguments = {} }; unshift @{$self-{queue}-{$vmid}}, $cmd; @@ -290,16 +303,17 @@ sub mux_close { # the descriptors. sub mux_input { my ($self, $mux, $fh, $input) = @_; - -if($self-{qga}){ - return if $$input !~ m/}\n(.+)}\n$/; -}else{ - return if $$input !~ m/}\r\n$/; -} -my $raw = $$input; +my $vmid = $self-{fhs_lookup}-{$fh}; +my $raw; +if ($self-{current}-{$vmid}-{qga}) { + return if $$input !~ s/^([^\n]+}\n[^\n]+})\n(.*)$/$2/so; + $raw = $1; +} else { + return if $$input !~ s/^([^\n]+})\r?\n(.*)$/$2/so; + $raw = $1; +} -my $vmid = $self-{fhs_lookup}-{$fh}; if (!$vmid) { warn internal error - unable to lookup vmid; return; @@ -308,7 +322,7 @@ sub mux_input { eval { my @jsons = split(\n, $raw); - if($self-{qga}){ + if($self-{current}-{$vmid}-{qga}){ die response is not complete if @jsons != 2 ; @@ -328,7 +342,7 @@ sub mux_input { $obj = from_json($jsons[1]); if (my $callback = $curcmd-{callback}) { - $callback($vmid, $obj); + $callback($vmid, $obj); } return; diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm index d740564..ab02b93 100644 --- a/PVE/QemuServer.pm +++ b/PVE/QemuServer.pm @@ -2881,13 +2881,6 @@ sub qga_socket { return ${var_run_tmpdir}/$vmid.qga; } -sub vm_qga_cmd { -my ($vmid, $execute, %params) = @_; - -my $cmd = { execute = $execute, arguments = \%params }; -vm_qmp_command($vmid, $cmd, undef, 1); -} - sub pidfile_name { my ($vmid) = @_; return ${var_run_tmpdir}/$vmid.pid; @@ -3481,7 +3474,7 @@ sub vm_mon_cmd_nocheck { } sub vm_qmp_command { -my ($vmid, $cmd, $nocheck, $qga) = @_; +my ($vmid, $cmd, $nocheck) = @_; my $res; @@ -3493,12 +3486,13 @@ sub vm_qmp_command { eval { die VM $vmid not running\n if !check_running($vmid, $nocheck); - my $sname = qmp_socket($vmid, $qga); + my $qga = ($cmd-{execute} =~ /^guest\-+/)?1:0; + my $sname = qmp_socket($vmid,$qga); if (-e $sname) { - my $qmpclient = PVE::QMPClient-new(undef, $qga); + my $qmpclient = PVE::QMPClient-new();
Re: [pve-devel] [PATCH 2/2] now if the QMP command starts with guest-+ , it will bind dynamicly to the VMID.qga socket. To test the function vmtime is implemented which return the vm UNIX Time.
First, thanks for the patch! Further comments inline: Signed-off-by: Wolfgang Link wolfg...@linksystems.org --- PVE/QMPClient.pm | 64 +++--- --- PVE/QemuServer.pm | 32 +++ qm| 31 ++ 3 files changed, 86 insertions(+), 41 deletions(-) diff --git a/PVE/QMPClient.pm b/PVE/QMPClient.pm index 9674d00..ab32a39 100755 --- a/PVE/QMPClient.pm +++ b/PVE/QMPClient.pm @@ -20,7 +20,7 @@ use Data::Dumper; # Note: kvm can onyl handle 1 connection, so we close connections asap sub new { -my ($class, $eventcb, $qga) = @_; +my ($class, $eventcb) = @_; my $mux = new IO::Multiplex; @@ -34,7 +34,6 @@ sub new { }, $class; $self-{eventcb} = $eventcb if $eventcb; -$self-{qga} = $qga if $qga; $mux-set_callback_object($self); @@ -124,10 +123,12 @@ my $close_connection = sub { }; my $open_connection = sub { -my ($self, $vmid, $timeout) = @_; - -my $sname = PVE::QemuServer::qmp_socket($vmid, $self-{qga}); +my ($self, $vmid, $timeout, $qga) = @_; +my $sname = PVE::QemuServer::qmp_socket($vmid); + +$sname = PVE::QemuServer::qga_socket($vmid) if $qga; I would simply use: my $sname = PVE::QemuServer::qmp_socket($vmid, $qga); + $timeout = 1 if !$timeout; my $fh; @@ -191,19 +192,21 @@ my $check_queue = sub { my $qmpcmd = undef; - if($self-{qga}){ + if($self-{current}-{$vmid}-{qga}){ + + my $qmpcmdid = undef; - my $qmpcmdid =to_json({ + $qmpcmdid =to_json({ This change is not really required? execute = 'guest-sync', - arguments = { id = int($cmd-{id})}}); - + arguments = { id = int ($cmd-{id}) }}); what is the difference? + $qmpcmd = to_json({ execute = $cmd-{execute}, arguments = $cmd-{arguments}}); - + another nop? $qmpcmd = $qmpcmdid.$qmpcmd; - }else{ + } else{ If you want to correct coding style, this should be + } else { But in general, it is better to do that with a separate patch. $qmpcmd = to_json({ execute = $cmd-{execute}, @@ -243,12 +246,17 @@ sub queue_execute { foreach my $vmid (keys %{$self-{queue}}) { next if !scalar(@{$self-{queue}-{$vmid}}); # no commands for the VM + if ($self-{queue}-{$vmid}[0]-{execute} =~ /^guest\-+/){ + $self-{queue}-{$vmid}[0]-{qga} = 1; + } + eval { - my $fh = $open_connection($self, $vmid, $timeout); - - if(!$self-{qga}){ - my $cmd = { execute = 'qmp_capabilities', arguments = {} }; - unshift @{$self-{queue}-{$vmid}}, $cmd; + my $fh = $open_connection($self, $vmid, $timeout, +$self-{queue}-{$vmid}[0]-{qga} ); + + if (!$self-{queue}-{$vmid}[0]-{qga}){ + my $cmd = { execute = 'qmp_capabilities', arguments = {} }; + unshift @{$self-{queue}-{$vmid}}, $cmd; + } $self-{mux}-set_timeout($fh, $timeout); }; @@ -256,8 +264,8 @@ sub queue_execute { warn $err; $self-{errors}-{$vmid} = $err; } - } white-space change } + white-space change my $running; for (;;) { @@ -290,16 +298,18 @@ sub mux_close { # the descriptors. sub mux_input { my ($self, $mux, $fh, $input) = @_; - -if($self-{qga}){ - return if $$input !~ m/}\n(.+)}\n$/; -}else{ - return if $$input !~ m/}\r\n$/; -} + +my $vmid = $self-{fhs_lookup}-{$fh}; -my $raw = $$input; +my $raw; +if ($self-{current}-{$vmid}-{qga}) { + return if $$input !~ s/^(.+}\n.+})\n(.*)$/$2/so; Maybe we can be more restrictive here: return if $$input !~ s/^([^\n]+}\n[^\n]+})\n(.*)$/$2/so; + $raw = $1; +} else { + return if $$input !~ s/^(.*})\r?\n(.*)$/$2/so; same here: return if $$input !~ s/^([^\n]+})\r?\n(.*)$/$2/so; + $raw = $1; +} -my $vmid = $self-{fhs_lookup}-{$fh}; if (!$vmid) { warn internal error - unable to lookup vmid; return; @@ -308,7 +318,7 @@ sub mux_input { eval { my @jsons = split(\n, $raw); - if($self-{qga}){ + if($self-{current}-{$vmid}-{qga}){ die response is not complete if @jsons != 2 ; @@ -321,7 +331,7 @@ sub mux_input { delete $self-{current}-{$vmid}; - if ($curcmd-{id} ne $cmdid) { + if (int ($curcmd-{id}) ne $cmdid) { why is this required? die got wrong command id '$cmdid' (expected $curcmd- {id})\n; } diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm index d740564..4709382 100644 --- a/PVE/QemuServer.pm +++ b/PVE/QemuServer.pm @@ -2871,9 +2871,8 @@ sub spice_port { } sub qmp_socket {
Re: [pve-devel] [PATCH 2/2] now if the QMP command starts with guest-+ , it will bind dynamicly to the VMID.qga socket. To test the function vmtime is implemented which return the vm UNIX Time.
Just reading: http://wiki.qemu.org/Features/QAPI/GuestAgent I wonder what happened to the qga_proxy feature? Is that still not implemented? ___ pve-devel mailing list pve-devel@pve.proxmox.com http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH 2/2] now if the QMP command starts with guest-+ , it will bind dynamicly to the VMID.qga socket. To test the function vmtime is implemented which return the vm UNIX Time.
I wonder what happened to the qga_proxy feature? Is that still not implemented? No, It's never has been implemented. Sigh! So we always run into timeouts if guest-agent is not running, and there is no way to avoid that? ___ pve-devel mailing list pve-devel@pve.proxmox.com http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH 2/2] now if the QMP command starts with guest-+ , it will bind dynamicly to the VMID.qga socket. To test the function vmtime is implemented which return the vm UNIX Time.
Sigh! So we always run into timeouts if guest-agent is not running, and there is no way to avoid that? I think that libvirt sent always guest-sync before doing the real command, to see the if guest-agent is alive. So, I think we can sent guest-sync with a real small timeout, like 1s, before doing a longer qga command which need a bigger timeout. - Mail original - De: Dietmar Maurer diet...@proxmox.com À: Alexandre DERUMIER aderum...@odiso.com Cc: pve-devel@pve.proxmox.com, Wolfgang Link wolfg...@linksystems.org Envoyé: Lundi 24 Novembre 2014 17:24:02 Objet: RE: [PATCH 2/2] now if the QMP command starts with guest-+ , it will bind dynamicly to the VMID.qga socket. To test the function vmtime is implemented which return the vm UNIX Time. I wonder what happened to the qga_proxy feature? Is that still not implemented? No, It's never has been implemented. Sigh! So we always run into timeouts if guest-agent is not running, and there is no way to avoid that? ___ pve-devel mailing list pve-devel@pve.proxmox.com http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH 2/2] now if the QMP command starts with guest-+ , it will bind dynamicly to the VMID.qga socket. To test the function vmtime is implemented which return the vm UNIX Time.
I think that libvirt sent always guest-sync before doing the real command, to see the if guest-agent is alive. So, I think we can sent guest-sync with a real small timeout, like 1s, before doing a longer qga command which need a bigger timeout. Ah, OK. Or we can save the state of the last command into a temp file, so that we can avoid the delay? if (last_command_was_successful()) { run_qmp_command() } else { test_conn_with_small_timeout() run_qmp_command() } ___ pve-devel mailing list pve-devel@pve.proxmox.com http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH 2/2] now if the QMP command starts with guest-+ , it will bind dynamicly to the VMID.qga socket. To test the function vmtime is implemented which return the vm UNIX Time.
Ah, OK. Or we can save the state of the last command into a temp file, so that we can avoid the delay? I'm not sure, because It's possible that qga daemon can be stopped, or crash, - Mail original - De: Dietmar Maurer diet...@proxmox.com À: Alexandre DERUMIER aderum...@odiso.com Cc: pve-devel@pve.proxmox.com, Wolfgang Link wolfg...@linksystems.org Envoyé: Lundi 24 Novembre 2014 18:00:24 Objet: RE: [PATCH 2/2] now if the QMP command starts with guest-+ , it will bind dynamicly to the VMID.qga socket. To test the function vmtime is implemented which return the vm UNIX Time. I think that libvirt sent always guest-sync before doing the real command, to see the if guest-agent is alive. So, I think we can sent guest-sync with a real small timeout, like 1s, before doing a longer qga command which need a bigger timeout. Ah, OK. Or we can save the state of the last command into a temp file, so that we can avoid the delay? if (last_command_was_successful()) { run_qmp_command() } else { test_conn_with_small_timeout() run_qmp_command() } ___ pve-devel mailing list pve-devel@pve.proxmox.com http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH 2/2] now if the QMP command starts with guest-+ , it will bind dynamicly to the VMID.qga socket. To test the function vmtime is implemented which return the vm UNIX Time.
Ah, OK. Or we can save the state of the last command into a temp file, so that we can avoid the delay? I'm not sure, because It's possible that qga daemon can be stopped, or crash, Right, that does not really help. Instead, the guest-agent should send some kind of 'alive' signal every second, which we could track inside qemu? ___ pve-devel mailing list pve-devel@pve.proxmox.com http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH 2/2] now if the QMP command starts with guest-+ , it will bind dynamicly to the VMID.qga socket. To test the function vmtime is implemented which return the vm UNIX Time.
On Mon, 24 Nov 2014 17:52:29 + Dietmar Maurer diet...@proxmox.com wrote: Right, that does not really help. Instead, the guest-agent should send some kind of 'alive' signal every second, which we could track inside qemu? Could some listening on qmp event be used? -- Hilsen/Regards Michael Rasmussen Get my public GnuPG keys: michael at rasmussen dot cc http://pgp.mit.edu:11371/pks/lookup?op=getsearch=0xD3C9A00E mir at datanom dot net http://pgp.mit.edu:11371/pks/lookup?op=getsearch=0xE501F51C mir at miras dot org http://pgp.mit.edu:11371/pks/lookup?op=getsearch=0xE3E80917 -- /usr/games/fortune -es says: In Pocataligo, Georgia, it is a violation for a woman over 200 pounds and attired in shorts to pilot or ride in an airplane. pgpRYhHieX9gl.pgp Description: OpenPGP digital signature ___ pve-devel mailing list pve-devel@pve.proxmox.com http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH 2/2] now if the QMP command starts with guest-+ , it will bind dynamicly to the VMID.qga socket. To test the function vmtime is implemented which return the vm UNIX Time.
Right, that does not really help. Instead, the guest-agent should send some kind of 'alive' signal every second, which we could track inside qemu? Could some listening on qmp event be used? You need to have an open connection to do that. I guess the best way to solve that problem is to implement the qga-proxy inside qemu. ___ pve-devel mailing list pve-devel@pve.proxmox.com http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH 2/2] now if the QMP command starts with guest-+ , it will bind dynamicly to the VMID.qga socket. To test the function vmtime is implemented which return the vm UNIX Time.
Right, that does not really help. Instead, the guest-agent should send some kind of 'alive' signal every second, which we could track inside qemu? What do we want to do you qga ? if it's only to send some commands like filesystem freeze|unfreeze, stop[start , I think that calling guest-sync before it's not a big overhead. - Mail original - De: Dietmar Maurer diet...@proxmox.com À: Alexandre DERUMIER aderum...@odiso.com Cc: pve-devel@pve.proxmox.com, Wolfgang Link wolfg...@linksystems.org Envoyé: Lundi 24 Novembre 2014 18:52:29 Objet: RE: [PATCH 2/2] now if the QMP command starts with guest-+ , it will bind dynamicly to the VMID.qga socket. To test the function vmtime is implemented which return the vm UNIX Time. Ah, OK. Or we can save the state of the last command into a temp file, so that we can avoid the delay? I'm not sure, because It's possible that qga daemon can be stopped, or crash, Right, that does not really help. Instead, the guest-agent should send some kind of 'alive' signal every second, which we could track inside qemu? ___ pve-devel mailing list pve-devel@pve.proxmox.com http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH 2/2] now if the QMP command starts with guest-+ , it will bind dynamicly to the VMID.qga socket. To test the function vmtime is implemented which return the vm UNIX Time.
What do we want to do you qga ? 1.) send guest command like shutdown, freeze, unfreeze 2.) query guest status if it's only to send some commands like filesystem freeze|unfreeze, stop[start , I think that calling guest-sync before it's not a big overhead. The problem is the timeout if guest agent is not running (not the overhead). But maybe it is not a big problem, and not worth the efforts? ___ pve-devel mailing list pve-devel@pve.proxmox.com http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH 2/2] now if the QMP command starts with guest-+ , it will bind dynamicly to the VMID.qga socket. To test the function vmtime is implemented which return the vm UNIX Time.
The problem is the timeout if guest agent is not running (not the overhead). But maybe it is not a big problem, and not worth the efforts? 1.) send guest command like shutdown, freeze, unfreeze I think it's not a problem here. a small extra 1s timeout on guest-sync is ok I Think. (and if guest-sync timeout, we can fallback on old method (acpi shutdown, vm pause) 2.) query guest status timeout could be a problem in pvestatd (not sure where you want to query guest status) - Mail original - De: Dietmar Maurer diet...@proxmox.com À: Alexandre DERUMIER aderum...@odiso.com Cc: pve-devel@pve.proxmox.com, Wolfgang Link wolfg...@linksystems.org Envoyé: Lundi 24 Novembre 2014 20:54:01 Objet: RE: [PATCH 2/2] now if the QMP command starts with guest-+ , it will bind dynamicly to the VMID.qga socket. To test the function vmtime is implemented which return the vm UNIX Time. What do we want to do you qga ? 1.) send guest command like shutdown, freeze, unfreeze 2.) query guest status if it's only to send some commands like filesystem freeze|unfreeze, stop[start , I think that calling guest-sync before it's not a big overhead. The problem is the timeout if guest agent is not running (not the overhead). But maybe it is not a big problem, and not worth the efforts? ___ pve-devel mailing list pve-devel@pve.proxmox.com http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH 2/2] now if the QMP command starts with guest-+ , it will bind dynamicly to the VMID.qga socket. To test the function vmtime is implemented which return the vm UNIX Time.
2.) query guest status timeout could be a problem in pvestatd (not sure where you want to query guest status) Yes, I thought about doing this in pvestatd. Maybe pvestatd can save if last qga command was successful (guest agent online). If not, it just sends 'guest-sync' with short timeout. ___ pve-devel mailing list pve-devel@pve.proxmox.com http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel