[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.
Dietmar Maurer
dietmar at proxmox.com
Mon Nov 24 16:59:53 CET 2014
First, thanks for the patch!
Further comments inline:
> Signed-off-by: Wolfgang Link <wolfgang at 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 {
> - my ($vmid, $qga) = @_;
> - my $sockettype = $qga ? 'qga' : 'qmp';
> - return "${var_run_tmpdir}/$vmid.$sockettype";
> + my ($vmid) = @_;
> + return "${var_run_tmpdir}/$vmid.qmp";
> }
There is no need to change that.
>
> sub qga_socket {
> @@ -2881,13 +2880,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"; @@ -3466,6 +3458,18 @@ sub
> vm_start {
> });
> }
>
> +sub get_vmtime {
> +
> + my ($vmid) = @_;
> +
> + my $qmpclient = PVE::QMPClient->new();
> +
> + my $cmd = {'execute' => 'guest-get-time'} ;
> +
> + return $qmpclient->cmd($vmid, $cmd); }
> +
> +
Please use a separate patch to implement 'vmtime' API.
> sub vm_mon_cmd {
> my ($vmid, $execute, %params) = @_;
>
> @@ -3481,7 +3485,7 @@ sub vm_mon_cmd_nocheck { }
>
> sub vm_qmp_command {
> - my ($vmid, $cmd, $nocheck, $qga) = @_;
> + my ($vmid, $cmd, $nocheck) = @_;
>
> my $res;
>
> @@ -3493,12 +3497,12 @@ sub vm_qmp_command {
>
> eval {
> die "VM $vmid not running\n" if !check_running($vmid, $nocheck);
> - my $sname = qmp_socket($vmid, $qga);
> + my $sname = qmp_socket($vmid);
> if (-e $sname) {
> - my $qmpclient = PVE::QMPClient->new(undef, $qga);
> + my $qmpclient = PVE::QMPClient->new();
>
> $res = $qmpclient->cmd($vmid, $cmd, $timeout);
> - } elsif (-e "${var_run_tmpdir}/$vmid.mon" && !$qga) {
> + } elsif (-e "${var_run_tmpdir}/$vmid.mon") {
> die "can't execute complex command on old monitor - stop/start your
> vm to fix the problem\n"
> if scalar(%{$cmd->{arguments}});
> vm_monitor_command($vmid, $cmd->{execute}, $nocheck); diff --git
> a/qm b/qm index cea223e..1af9550 100755
> --- a/qm
> +++ b/qm
> @@ -44,6 +44,11 @@ my $upid_exit = sub {
>
> my $nodename = PVE::INotify::nodename();
>
> +my $dump_output = sub {
> + my $output = shift;
> + print $output."\n";
> +};
> +
> sub run_vnc_proxy {
> my ($path) = @_;
>
> @@ -387,6 +392,30 @@ __PACKAGE__->register_method ({
> return undef;
> }});
>
> +__PACKAGE__->register_method ({
> + name => 'vmtime',
> + path => 'vmtime',
> + method => 'GET',
> + description => "Get the systemtime from the VMID vm')",
> + parameters => {
> + additionalProperties => 0,
> + properties => {
> + vmid => get_standard_option('pve-vmid'),
> + },
> + },
> + returns => { type => 'string'},
> + code => sub {
> + my ($param) = @_;
> +
> + my $vmid = $param->{vmid};
> +
> + my $conf = PVE::QemuServer::load_config ($vmid); # check if VM exists
> +
> + die "VM $vmid is not running\n" if
> +!PVE::QemuServer::check_running($vmid);
> +
> + return PVE::QemuServer::get_vmtime($vmid);
> + }});
> +
> my $cmddef = {
> list => [ "PVE::API2::Qemu", 'vmlist', [],
> { node => $nodename }, sub {
> @@ -476,6 +505,8 @@ my $cmddef = {
> mtunnel => [ __PACKAGE__, 'mtunnel', []],
>
> terminal => [ __PACKAGE__, 'terminal', ['vmid']],
> +
> + vmtime => [ __PACKAGE__, 'vmtime', ['vmid'], {}, $dump_output],
> };
>
> my $cmd = shift;
> --
> 1.7.10.4
>
More information about the pve-devel
mailing list