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
[pve-devel] vm_deviceunplug question
For example, for virtio we call qemu_devicedelverify():
if ($deviceid =~ m/^(virtio)(\d+)$/) {
qemu_devicedel($vmid, $deviceid);
return undef if !qemu_devicedelverify($vmid, $deviceid);
return undef if !qemu_drivedel($vmid, $deviceid);
But for lsi/scsi we simply do:
if ($deviceid =~ m/^(scsi)(\d+)$/) {
qemu_devicedel($vmid, $deviceid);
return undef if !qemu_drivedel($vmid, $deviceid);
Why don't we call qemu_devicedelverify() here?
___
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
Re: [pve-devel] [PATCH v4 14/14] implement API/CLI to get pending changes
I don't known why, but I can't fill the datastore with a simple model like
Ext.define('pendingconf', {
extend: Ext.data.Model,
fields: [ 'key', 'value', 'pending', 'delete' ],
idProperty: 'key'
});
Do you get some kind of error message?
___
pve-devel mailing list
pve-devel@pve.proxmox.com
http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH v4 14/14] implement API/CLI to get pending changes
Do you get some kind of error message?
no, no error.
But I think my problem is that I used the jsonobject reader, which only work
with key-value
I dig a little bit today. (Damn, I don't like js ;)
- Mail original -
De: Dietmar Maurer diet...@proxmox.com
À: Alexandre DERUMIER aderum...@odiso.com
Cc: pve-devel@pve.proxmox.com
Envoyé: Mardi 25 Novembre 2014 08:11:22
Objet: RE: [pve-devel] [PATCH v4 14/14] implement API/CLI to get pending
changes
I don't known why, but I can't fill the datastore with a simple model like
Ext.define('pendingconf', {
extend: Ext.data.Model,
fields: [ 'key', 'value', 'pending', 'delete' ],
idProperty: 'key'
});
Do you get some kind of error message?
___
pve-devel mailing list
pve-devel@pve.proxmox.com
http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
