Re: [Xen-devel] [OSSTEST PATCH 1/2] libvirt: Check migration capabilities using proper XML parser

[Jim Fehlig <jfehlig@xxxxxxxx>]

On 10/04/2016 11:02 AM, Ian Jackson wrote:
> Do not grep the virsh capabilities output (!)  Instead, parse the XML
> using perl's XML modules and look for the specific feature flag using
> an XPATH pattern.
>
> AFAICT from looking at the XML, that's
>
>     <capabilities>
>       <host>
>         <migration_features>
>           <live/>
>
> But the original code does not test for <live/>.
>
> Xen could in principle (and AIUI might in the future, on ARM) support
> save/restore but not live migration.  Currently it supports neither.
>
> I don't know whether libvirt's capabilities system can capture this
> distinction.  libvirt.git#1d37a4c4 "libxl: detect support for save and
> restore" suggests not.
That's correct. But as Martin suggested in 2/2, libvirt could be enhanced to 
report the distinction if/when needed.
> Perhaps relatedly, I am not sure whether this test should be changed
> to look for the xpath
>   /capabilities/host/migration_features/live
> instead.  The schema (libvirt.git/docs/schemas/capability.rng) seems
> to suggest that it probably should.
Agreed. migrate_check() should look for
/capabilities/host/migration_features/live.

Regards,
Jim

> For now, this osstest commit has no ultimate functional change (with
> libvirt output as it currently appears to be on real hosts).
>
> Signed-off-by: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
> CC: Julien Grall <julien.grall@xxxxxxx>
> CC: Jim Fehlig <jfehlig@xxxxxxxx>
> ---
>  Osstest/Toolstack/libvirt.pm | 17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/Osstest/Toolstack/libvirt.pm b/Osstest/Toolstack/libvirt.pm
> index 69ff0bb..b7db7af 100644
> --- a/Osstest/Toolstack/libvirt.pm
> +++ b/Osstest/Toolstack/libvirt.pm
> @@ -21,6 +21,8 @@ use strict;
>  use warnings;
>
>  use Osstest::TestSupport;
> +use XML::LibXML::XPathContext;
> +use XML::LibXML;
>
>  sub new {
>      my ($class, $ho, $methname,$asset) = @_;
> @@ -72,6 +74,17 @@ sub shutdown_wait ($$$) {
>      guest_await_destroy($gho,$timeout);
>  }
>
> +sub _check_capability ($$) {
> +    my ($self, $xpath) = @_;
> +    my $ho = $self->{Host};
> +    my $caps = target_cmd_output_root($ho, 'virsh capabilities');
> +    my $stash = open_unique_stashfile('virsh-capabilities');
> +    my $dom = XML::LibXML->load_xml(string => $caps);
> +    my $xc = XML::LibXML::XPathContext->new($dom);
> +    my @nodes = $xc->findnodes($xpath);
> +    return !!@nodes;
> +}
> +
>  sub migrate_check ($$) {
>      my ($self, $local) = @_;
>      my $rc;
> @@ -80,9 +93,7 @@ sub migrate_check ($$) {
>          # local migration is not supported
>          $rc = 1;
>      } else {
> -       my $ho = $self->{Host};
> -       my $caps = target_cmd_output_root($ho, "virsh capabilities");
> -       $rc = ($caps =~ m/<migration_features>/) ? 0 : 1
> +       $rc = $self->check_capability('/capabilities/host/migration_features');
>      }
>
>      logm("rc=$rc");

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel