[libvirt] [perl-Sys-Virt][PATCH] Virt.xs: fix flag issue on set_scheduler_parameters
From: Alex Jia <Alex Jia ajia@redhat.com> 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@redhat.com> --- 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
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@redhat.com> To: libvir-list@redhat.com Cc: "Alex Jia" <Alex.Jia.ajia@redhat.com> 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@redhat.com> 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@redhat.com> --- 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/28/2012 11:38 AM, Alex Jia wrote:
From: Alex Jia <Alex Jia ajia@redhat.com>
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.
Yes, it is a bug.
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@redhat.com> --- 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)
How about setting flags = -1 as default value. set_scheduler_parameters(dom, newparams, flags=-1) int flags;
} } vir_typed_param_from_hv(newparams, params, nparams); - if (flags) { + if (flags != 1) {
if (flags < 0) { virDomainSetSchedulerParameters() } else { virDomainSetSchedulerParametersFlags(); }
if (virDomainSetSchedulerParametersFlags(dom, params, nparams, flags) < 0) _croak_error(); } else {
Your code can work but a little weird in semantics. Guannan
On 08/29/2012 02:12 PM, Guannan Ren wrote:
On 08/28/2012 11:38 AM, Alex Jia wrote:
From: Alex Jia <Alex Jia ajia@redhat.com>
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.
Yes, it is a bug.
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@redhat.com> --- 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)
How about setting flags = -1 as default value. set_scheduler_parameters(dom, newparams, flags=-1) int flags;
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
} } vir_typed_param_from_hv(newparams, params, nparams); - if (flags) { + if (flags != 1) {
if (flags < 0) { virDomainSetSchedulerParameters() } else { virDomainSetSchedulerParametersFlags(); }
if (virDomainSetSchedulerParametersFlags(dom, params, nparams, flags) < 0) _croak_error(); } else {
Your code can work but a little weird in semantics.
Guannan
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 08/29/2012 02:56 AM, Alex Jia wrote:
On 08/29/2012 02:12 PM, Guannan Ren wrote:
On 08/28/2012 11:38 AM, Alex Jia wrote:
From: Alex Jia <Alex Jia ajia@redhat.com>
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.
+++ b/Virt.xs @@ -2833,7 +2833,7 @@ set_scheduler_parameters(dom, newparams, flags=0)
How about setting flags = -1 as default value. set_scheduler_parameters(dom, newparams, flags=-1) int flags;
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.
That's not quite correct - remember, virDomainSetSchedulerParametersFlags() is a newer function, and when talking to older libvirtd, it did not work when the older function virDomainSetSchedulerParameters() will work. Thus, if flags is 0, you should call the older function, for a better chance of working across more setups. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (3)
-
Alex Jia -
Eric Blake -
Guannan Ren