Re: [libvirt] [OSSTEST PATCH 2/2] libvirt: Do not attempt save/restore when migration not advertised

2016-10-06 Thread Ian Jackson
Martin Kletzander writes ("Re: [OSSTEST PATCH 2/2] libvirt: Do not attempt 
save/restore when migration not advertised"):
> Well then, unfortunately you do.
> 
> Also, looking at how the code is structured, if you have live migration
> but don't have save/restore, you won't have  there
> at all.

Right.  OK, thanks.  I will add the patch below to my osstest queue.

Ian.

>From 5330ff9222e4e611505149945eef7dc074b4f9b5 Mon Sep 17 00:00:00 2001
In-Reply-To: <20161006104255.GP16414@wheatley>
References: <20161006104255.GP16414@wheatley>
From: Ian Jackson 
Date: Thu, 6 Oct 2016 17:38:29 +0100
Subject: [OSSTEST PATCH 3/2] libvirt: Check 
/capabilities/host/migration_features/live for live migration
Cc: libvir-list@redhat.com

libvirt is capable of advertising this separately from
/capabilities/host/migration_features, so if save/restore is supported
but live migration is not, this will do the right thing.

We would have preferred libvirt to advertise
  /capabilities/host/migration_features/save
or something, but it doesn't right now, so we continue to use
  /capabilities/host/migration_features
to detect save/restore support.

If libvirt changes its feature presentation, then at some future point
we should change osstest too.

Signed-off-by: Ian Jackson 
CC: Martin Kletzander 
CC: Jim Fehlig 
---
 Osstest/Toolstack/libvirt.pm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Osstest/Toolstack/libvirt.pm b/Osstest/Toolstack/libvirt.pm
index 250fe47..81e724d 100644
--- a/Osstest/Toolstack/libvirt.pm
+++ b/Osstest/Toolstack/libvirt.pm
@@ -93,7 +93,8 @@ sub migrate_check ($$) {
 # local migration is not supported
 $rc = 1;
 } else {
-   $rc = $self->check_capability('/capabilities/host/migration_features');
+   $rc = $self->check_capability
+   ('/capabilities/host/migration_features/live');
 }
 
 logm("rc=$rc");
-- 
2.1.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [OSSTEST PATCH 2/2] libvirt: Do not attempt save/restore when migration not advertised

2016-10-06 Thread Martin Kletzander

On Thu, Oct 06, 2016 at 10:59:06AM +0100, Ian Jackson wrote:

Martin Kletzander writes ("Re: [OSSTEST PATCH 2/2] libvirt: Do not attempt 
save/restore when migration not advertised"):

Since offline migration (as in migrating a domain between hosts without
being running) is not that used in the code and talked about, I'm
guessing offline means save restore.  Looking at the history it was
added before the "offline" migration, so it probably means
save/restore.  To avoid confusion, I would suggest we add either
 or rather  (the naming is not important) and document
what it means.  And then you can use it exactly how you'd like.  And
you'll be also sure it means what you need it to mean ;)  The patches
will be straigh-forward, let me know if I can help anyhow.


Except that the point of the exercise is to detect which features are
supported in which versions.  Whatever I do in osstest needs to work
with older libvirt versions, which do not report
 /capabilities/host/migration_features/save
even on x86, where it is supported.  I suppose I could detect
 /capabilities/host/migration_features/live
and assume that save/restore was supported (since it's unlikely that
live migration would be supported but not save/restore).

So for now I think I need to use
 /capabilities/host/migration_features
as a proxy for save/restore ?



Well then, unfortunately you do.

Also, looking at how the code is structured, if you have live migration
but don't have save/restore, you won't have  there
at all.


Ian.


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [OSSTEST PATCH 2/2] libvirt: Do not attempt save/restore when migration not advertised

2016-10-06 Thread Ian Jackson
Martin Kletzander writes ("Re: [OSSTEST PATCH 2/2] libvirt: Do not attempt 
save/restore when migration not advertised"):
> Since offline migration (as in migrating a domain between hosts without
> being running) is not that used in the code and talked about, I'm
> guessing offline means save restore.  Looking at the history it was
> added before the "offline" migration, so it probably means
> save/restore.  To avoid confusion, I would suggest we add either
>  or rather  (the naming is not important) and document
> what it means.  And then you can use it exactly how you'd like.  And
> you'll be also sure it means what you need it to mean ;)  The patches
> will be straigh-forward, let me know if I can help anyhow.

Except that the point of the exercise is to detect which features are
supported in which versions.  Whatever I do in osstest needs to work
with older libvirt versions, which do not report
  /capabilities/host/migration_features/save
even on x86, where it is supported.  I suppose I could detect
  /capabilities/host/migration_features/live
and assume that save/restore was supported (since it's unlikely that
live migration would be supported but not save/restore).

So for now I think I need to use
  /capabilities/host/migration_features
as a proxy for save/restore ?

Ian.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [OSSTEST PATCH 2/2] libvirt: Do not attempt save/restore when migration not advertised

2016-10-06 Thread Martin Kletzander

On Wed, Oct 05, 2016 at 06:06:29PM -0600, Jim Fehlig wrote:

On 10/04/2016 11:02 AM, Ian Jackson wrote:

Currently, osstest wrongly thinks that ARM can do save/restore,
because `virsh help' does mention the save command (on all
architectures).

Additionally, check the virth capabilities xpath
   /capabilities/host/migration_features
to try to see whether this host supports migration.

I am not sure if this is the right path to check.  Perhaps
   /capabilities/host/migration_features/live
is more correct, but this may be wrong if Xen comes to support save/restore
on ARM, but not live migration (but perhaps libvirt cannot express this
distinction in which case perhaps it's right after all).


Looking at the capabilities generation code again, I see that
virCapabilitiesNew() takes 'offlineMigrate' and 'liveMigrate' parameters. I
assume offline in this context means save, copy, restore. Martin, is that
assumption correct?



The thing is that it's not documented.  I can't even say "enough", it's
more like "at all".  You can have a look at it:

 https://libvirt.org/formatcaps.html

It doesn't even talk about , just , but
I guess that's the same thing.

Since offline migration (as in migrating a domain between hosts without
being running) is not that used in the code and talked about, I'm
guessing offline means save restore.  Looking at the history it was
added before the "offline" migration, so it probably means
save/restore.  To avoid confusion, I would suggest we add either
 or rather  (the naming is not important) and document
what it means.  And then you can use it exactly how you'd like.  And
you'll be also sure it means what you need it to mean ;)  The patches
will be straigh-forward, let me know if I can help anyhow.


If so, I think the saverestore_check() below can look for
/capabilities/host/migration_features. The migration check in 1/2 can look for
/capabilities/host/migration_features/live. Is it fair to assume save/restore is
available when live migration is supported?



With that you could straight check for  and  ;)

Martin


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [OSSTEST PATCH 2/2] libvirt: Do not attempt save/restore when migration not advertised

2016-10-05 Thread Jim Fehlig

On 10/04/2016 11:02 AM, Ian Jackson wrote:

Currently, osstest wrongly thinks that ARM can do save/restore,
because `virsh help' does mention the save command (on all
architectures).

Additionally, check the virth capabilities xpath
   /capabilities/host/migration_features
to try to see whether this host supports migration.

I am not sure if this is the right path to check.  Perhaps
   /capabilities/host/migration_features/live
is more correct, but this may be wrong if Xen comes to support save/restore
on ARM, but not live migration (but perhaps libvirt cannot express this
distinction in which case perhaps it's right after all).


Looking at the capabilities generation code again, I see that 
virCapabilitiesNew() takes 'offlineMigrate' and 'liveMigrate' parameters. I 
assume offline in this context means save, copy, restore. Martin, is that 
assumption correct?


If so, I think the saverestore_check() below can look for
/capabilities/host/migration_features. The migration check in 1/2 can look for
/capabilities/host/migration_features/live. Is it fair to assume save/restore is 
available when live migration is supported?


Regards,
Jim



Signed-off-by: Ian Jackson 
CC: Julien Grall 
CC: Jim Fehlig 
---
 Osstest/Toolstack/libvirt.pm | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Osstest/Toolstack/libvirt.pm b/Osstest/Toolstack/libvirt.pm
index b7db7af..250fe47 100644
--- a/Osstest/Toolstack/libvirt.pm
+++ b/Osstest/Toolstack/libvirt.pm
@@ -111,7 +111,9 @@ sub check_for_command($$) {

 sub saverestore_check ($) {
 my ($self) = @_;
-return check_for_command($self, "save");
+return
+   _check_capability($self, '/capabilities/host/migration_features') &&
+   check_for_command($self, "save");
 }

 sub migrate () {



--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [OSSTEST PATCH 2/2] libvirt: Do not attempt save/restore when migration not advertised

2016-10-04 Thread Martin Kletzander

On Tue, Oct 04, 2016 at 06:02:27PM +0100, Ian Jackson wrote:

Currently, osstest wrongly thinks that ARM can do save/restore,
because `virsh help' does mention the save command (on all
architectures).

Additionally, check the virth capabilities xpath
  /capabilities/host/migration_features
to try to see whether this host supports migration.



I think this is pretty accurate.  At least for now.  I can't test the
code, but it looks fine.  Anyway, to stay in the safe waters, I'll just
comment the libvirt part ;)


I am not sure if this is the right path to check.  Perhaps
  /capabilities/host/migration_features/live
is more correct, but this may be wrong if Xen comes to support save/restore
on ARM, but not live migration (but perhaps libvirt cannot express this
distinction in which case perhaps it's right after all).



I think it does not matter for now.  If you add support for live
migrations, there will be both elements present in the XML, so whatever
you use you will face the same problem.  We should add a capability for
save/restore so that hypervisors, for which it's different thing than
migration, can distinguish that.  Maybe in the future we'll need to add
this per-guest, but I don't see the point right now.


Signed-off-by: Ian Jackson 
CC: Julien Grall 
CC: Jim Fehlig 
---
Osstest/Toolstack/libvirt.pm | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Osstest/Toolstack/libvirt.pm b/Osstest/Toolstack/libvirt.pm
index b7db7af..250fe47 100644
--- a/Osstest/Toolstack/libvirt.pm
+++ b/Osstest/Toolstack/libvirt.pm
@@ -111,7 +111,9 @@ sub check_for_command($$) {

sub saverestore_check ($) {
my ($self) = @_;
-return check_for_command($self, "save");
+return
+   _check_capability($self, '/capabilities/host/migration_features') &&
+   check_for_command($self, "save");
}

sub migrate () {
--
2.1.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list