In fact, the virsh schedinfo always calls virDomain{Get,Set}SchedulerParametersFlags() regardless what's flag, so I think we may also remove 'virDomainSetSchedulerParameters()' relevant branch. -- Regards, Alex ----- Original Message ----- From: "Alex Jia" <ajia(a)redhat.com&gt; To: libvir-list(a)redhat.com Cc: "Alex Jia" <Alex.Jia.ajia(a)redhat.com&gt; Sent: Tuesday, August 28, 2012 11:38:52 AM Subject: [libvirt][perl-Sys-Virt][PATCH] Virt.xs: fix flag issue on set_scheduler_parameters From: Alex Jia <Alex Jia ajia(a)redhat.com&gt; The default flags are inconsistent on both qemuSetSchedulerParameters() and qemuGetSchedulerParameters() in libvirt, the qemuGetSchedulerParameters() always passes 'VIR_DOMAIN_AFFECT_CURRENT' flag to the qemuGetSchedulerParametersFlags(), it should be a expected behavior, but the qemuSetSchedulerParameters() always passes 'VIR_DOMAIN_AFFECT_LIVE' flag to the qemuSetSchedulerParametersFlags(), if users use default flag=0 or explicitly give a 'VIR_DOMAIN_AFFECT_CURRENT' flag to the set_scheduler_parameters() in perl-Sys-Virt, because the flag value is 0, the result is the virDomainSetSchedulerParameters() is called incorrectly. How to reproduce? # cat test.pl #!/usr/bin/env perl use warnings; use strict; use Sys::Virt; my $uri = "qemu:///system"; my $domname = "foo"; # change your guest name my $con = Sys::Virt->new(address => $uri, readonly => 0); my $dom = $con->get_domain_by_name($domname); my %sched_param = (Sys::Virt::Domain::SCHEDULER_CPU_SHARES=>1); $dom->set_scheduler_parameters(\%sched_param, Sys::Virt::Domain::AFFECT_CURRENT); # perl test.pl libvirt error code: 55, message: Requested operation is not valid: domain is not running Signed-off-by: Alex Jia <Alex Jia ajia(a)redhat.com&gt; --- Virt.xs | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/Virt.xs b/Virt.xs index 2b8d74c..0ebf95d 100644 --- a/Virt.xs +++ b/Virt.xs @@ -2833,7 +2833,7 @@ set_scheduler_parameters(dom, newparams, flags=0) } } vir_typed_param_from_hv(newparams, params, nparams); - if (flags) { + if (flags != 1) { if (virDomainSetSchedulerParametersFlags(dom, params, nparams, flags) < 0) _croak_error(); } else { -- 1.7.1

On 08/29/2012 02:12 PM, Guannan Ren wrote: I don't want to change many codes, so just change condition judgement in here, in fact, it's enough to call virDomainSetSchedulerParametersFlags() regardless of what flag is like 'virsh schedinfo', hence, I trend to remove virDomainSetSchedulerParameters() relevant branch. Thanks, Alex