Re: [Xen-devel] [PATCH] libxl: Disallow save or migrate when host devices are assigned to a guest.
On Wed, 2015-05-06 at 11:59 -0600, Jim Fehlig wrote: Dario Faggioli wrote: On Fri, 2015-05-01 at 15:32 -0600, Jim Fehlig wrote: E.g. if a domain is configured to use memory allocated from certain numa nodes, we'd want to make sure the dest has sufficient memory amongst those nodes. But these were just examples of things we may want to check before starting the migration. I suppose there are better examples wrt storage, e.g. does the domain have an attached DVD, ... Do (if you know that) other drivers in libvirt do these kind of checks? Yes. The qemu driver has quite extensive migration checks. Interesting. I never had the chance to check that. I think we should have a look and try to figure out whether some o them make sense for us too, and decide whether we want them in libxl or in libvirt. I'm adding this to my TODO list. Thanks and Regards, Dario signature.asc Description: This is a digitally signed message part ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] libxl: Disallow save or migrate when host devices are assigned to a guest.
Dario Faggioli wrote: On Fri, 2015-05-01 at 15:32 -0600, Jim Fehlig wrote: Dario Faggioli wrote: I mean what NUMA configuration are we talking about, and when and under what circumstances would you consider it migratable? E.g. if a domain is configured to use memory allocated from certain numa nodes, we'd want to make sure the dest has sufficient memory amongst those nodes. Right. I was asking because I'm interested in the problem at the libxl level. To the best of my knowledge, I don't think we do anything like that in libxl for now, and I still am not sure whether we should... :-) But these were just examples of things we may want to check before starting the migration. I suppose there are better examples wrt storage, e.g. does the domain have an attached DVD, ... Exactly. Do (if you know that) other drivers in libvirt do these kind of checks? Yes. The qemu driver has quite extensive migration checks. Regards, Jim ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] libxl: Disallow save or migrate when host devices are assigned to a guest.
Dario Faggioli wrote:
On Tue, 2015-04-14 at 20:15 -0600, Jim Fehlig wrote:
Konrad Rzeszutek Wilk wrote:
-static bool
-libxlDomainMigrationIsAllowed(virDomainDefPtr def)
-{
-/* Migration is not allowed if definition contains any hostdevs */
-if (def-nhostdevs 0) {
-virReportError(VIR_ERR_OPERATION_INVALID, %s,
- _(domain has assigned host devices));
-return false;
-}
-
-return true;
-}
-
This function was created with the intention of adding more checks, e.g.
is the cpu and numa configuration migratable?
Only vaguely related to the patch, I know (sorry!), but if I can ask,
what do you mean with is the cpu and numa configuration migratable?
I was thinking about domXML such as
domain
...
vcpu cpuset='0-8'8/vcpu
memory62914560/memory
maxMemory104857600/maxMemory
numatune
memory mode='strict' nodeset='1-2'/
/numatune
...
/domain
I mean what NUMA configuration are we talking about, and when and under
what circumstances would you consider it migratable?
E.g. if a domain is configured to use memory allocated from certain numa
nodes, we'd want to make sure the dest has sufficient memory amongst
those nodes.
But these were just examples of things we may want to check before
starting the migration. I suppose there are better examples wrt
storage, e.g. does the domain have an attached DVD, ...
Regards,
Jim
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] libxl: Disallow save or migrate when host devices are assigned to a guest.
On Tue, 2015-04-14 at 20:15 -0600, Jim Fehlig wrote:
Konrad Rzeszutek Wilk wrote:
-static bool
-libxlDomainMigrationIsAllowed(virDomainDefPtr def)
-{
-/* Migration is not allowed if definition contains any hostdevs */
-if (def-nhostdevs 0) {
-virReportError(VIR_ERR_OPERATION_INVALID, %s,
- _(domain has assigned host devices));
-return false;
-}
-
-return true;
-}
-
This function was created with the intention of adding more checks, e.g.
is the cpu and numa configuration migratable?
Only vaguely related to the patch, I know (sorry!), but if I can ask,
what do you mean with is the cpu and numa configuration migratable?
I mean what NUMA configuration are we talking about, and when and under
what circumstances would you consider it migratable?
Thanks and Regards,
Dario
signature.asc
Description: This is a digitally signed message part
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] libxl: Disallow save or migrate when host devices are assigned to a guest.
On Tue, Apr 14, 2015 at 08:15:52PM -0600, Jim Fehlig wrote:
Konrad Rzeszutek Wilk wrote:
It is unhealthy. If the device is not doing any DMA operations
it would work - but if you are saving and there are DMA operations
happening the chance of corruption (outstanding DMAs) increase.
As such re-use the check migration used.
s/used// ?
nods
Signed-off-by: Konrad Rzeszutek Wilk konrad.w...@oracle.com
---
src/libxl/libxl_domain.c| 19 +++
src/libxl/libxl_domain.h| 3 +++
src/libxl/libxl_driver.c| 6 ++
src/libxl/libxl_migration.c | 15 +--
4 files changed, 29 insertions(+), 14 deletions(-)
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index 5e0ab56..c038989 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -178,6 +178,25 @@ libxlDomainObjEndJob(libxlDriverPrivatePtr driver
ATTRIBUTE_UNUSED,
return virObjectUnref(obj);
}
+/*
+ * Checks whether the domain has host devices (PCIe) to disallow
+ * migration or saving of guest - unless they are detached.
+ *
+ * Returns true if the domain has host devices, otherwise false.
+ */
+bool
+libxlDomainHasHostDevices(virDomainDefPtr def)
+{
+/* Migration is not allowed if definition contains any hostdevs */
+if (def-nhostdevs 0) {
+virReportError(VIR_ERR_OPERATION_INVALID, %s,
+ _(domain has assigned host devices));
+return true;
+}
+
+return false;
+}
+
static void *
libxlDomainObjPrivateAlloc(void)
{
diff --git a/src/libxl/libxl_domain.h b/src/libxl/libxl_domain.h
index aa647b8..76c0c8d 100644
--- a/src/libxl/libxl_domain.h
+++ b/src/libxl/libxl_domain.h
@@ -95,6 +95,9 @@ char *
libxlDomainManagedSavePath(libxlDriverPrivatePtr driver,
virDomainObjPtr vm);
+bool
+libxlDomainHasHostDevices(virDomainDefPtr def);
+
int
libxlDomainSaveImageOpen(libxlDriverPrivatePtr driver,
libxlDriverConfigPtr cfg,
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 9eb071e..b9faba8 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -1650,6 +1650,9 @@ libxlDomainSaveFlags(virDomainPtr dom, const char
*to, const char *dxml,
goto endjob;
}
+if (libxlDomainHasHostDevices(vm-def))
+goto endjob;
+
if (libxlDoDomainSave(driver, vm, to) 0)
goto endjob;
@@ -1876,6 +1879,9 @@ libxlDomainManagedSave(virDomainPtr dom, unsigned int
flags)
goto endjob;
}
+if (libxlDomainHasHostDevices(vm-def))
+goto endjob;
+
name = libxlDomainManagedSavePath(driver, vm);
if (name == NULL)
goto endjob;
diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c
index 4010506..aaed448c 100644
--- a/src/libxl/libxl_migration.c
+++ b/src/libxl/libxl_migration.c
@@ -209,19 +209,6 @@ libxlDoMigrateSend(libxlDriverPrivatePtr driver,
return ret;
}
-static bool
-libxlDomainMigrationIsAllowed(virDomainDefPtr def)
-{
-/* Migration is not allowed if definition contains any hostdevs */
-if (def-nhostdevs 0) {
-virReportError(VIR_ERR_OPERATION_INVALID, %s,
- _(domain has assigned host devices));
-return false;
-}
-
-return true;
-}
-
This function was created with the intention of adding more checks, e.g.
is the cpu and numa configuration migratable? Are there pending
snapshot or block jobs (once those are supported)? ...
I'd like to keep libxlDomainMigrationIsAllowed even though it currently
only checks for assigned hostdevs. I think it improves readability of
the migration code. For the time it could simply call
libxlDomainHasHostDevices.
You got it.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] libxl: Disallow save or migrate when host devices are assigned to a guest.
Konrad Rzeszutek Wilk wrote:
It is unhealthy. If the device is not doing any DMA operations
it would work - but if you are saving and there are DMA operations
happening the chance of corruption (outstanding DMAs) increase.
As such re-use the check migration used.
s/used// ?
Signed-off-by: Konrad Rzeszutek Wilk konrad.w...@oracle.com
---
src/libxl/libxl_domain.c| 19 +++
src/libxl/libxl_domain.h| 3 +++
src/libxl/libxl_driver.c| 6 ++
src/libxl/libxl_migration.c | 15 +--
4 files changed, 29 insertions(+), 14 deletions(-)
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index 5e0ab56..c038989 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -178,6 +178,25 @@ libxlDomainObjEndJob(libxlDriverPrivatePtr driver
ATTRIBUTE_UNUSED,
return virObjectUnref(obj);
}
+/*
+ * Checks whether the domain has host devices (PCIe) to disallow
+ * migration or saving of guest - unless they are detached.
+ *
+ * Returns true if the domain has host devices, otherwise false.
+ */
+bool
+libxlDomainHasHostDevices(virDomainDefPtr def)
+{
+/* Migration is not allowed if definition contains any hostdevs */
+if (def-nhostdevs 0) {
+virReportError(VIR_ERR_OPERATION_INVALID, %s,
+ _(domain has assigned host devices));
+return true;
+}
+
+return false;
+}
+
static void *
libxlDomainObjPrivateAlloc(void)
{
diff --git a/src/libxl/libxl_domain.h b/src/libxl/libxl_domain.h
index aa647b8..76c0c8d 100644
--- a/src/libxl/libxl_domain.h
+++ b/src/libxl/libxl_domain.h
@@ -95,6 +95,9 @@ char *
libxlDomainManagedSavePath(libxlDriverPrivatePtr driver,
virDomainObjPtr vm);
+bool
+libxlDomainHasHostDevices(virDomainDefPtr def);
+
int
libxlDomainSaveImageOpen(libxlDriverPrivatePtr driver,
libxlDriverConfigPtr cfg,
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 9eb071e..b9faba8 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -1650,6 +1650,9 @@ libxlDomainSaveFlags(virDomainPtr dom, const char *to,
const char *dxml,
goto endjob;
}
+if (libxlDomainHasHostDevices(vm-def))
+goto endjob;
+
if (libxlDoDomainSave(driver, vm, to) 0)
goto endjob;
@@ -1876,6 +1879,9 @@ libxlDomainManagedSave(virDomainPtr dom, unsigned int
flags)
goto endjob;
}
+if (libxlDomainHasHostDevices(vm-def))
+goto endjob;
+
name = libxlDomainManagedSavePath(driver, vm);
if (name == NULL)
goto endjob;
diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c
index 4010506..aaed448c 100644
--- a/src/libxl/libxl_migration.c
+++ b/src/libxl/libxl_migration.c
@@ -209,19 +209,6 @@ libxlDoMigrateSend(libxlDriverPrivatePtr driver,
return ret;
}
-static bool
-libxlDomainMigrationIsAllowed(virDomainDefPtr def)
-{
-/* Migration is not allowed if definition contains any hostdevs */
-if (def-nhostdevs 0) {
-virReportError(VIR_ERR_OPERATION_INVALID, %s,
- _(domain has assigned host devices));
-return false;
-}
-
-return true;
-}
-
This function was created with the intention of adding more checks, e.g.
is the cpu and numa configuration migratable? Are there pending
snapshot or block jobs (once those are supported)? ...
I'd like to keep libxlDomainMigrationIsAllowed even though it currently
only checks for assigned hostdevs. I think it improves readability of
the migration code. For the time it could simply call
libxlDomainHasHostDevices.
Regards,
Jim
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] [PATCH] libxl: Disallow save or migrate when host devices are assigned to a guest.
It is unhealthy. If the device is not doing any DMA operations
it would work - but if you are saving and there are DMA operations
happening the chance of corruption (outstanding DMAs) increase.
As such re-use the check migration used.
Signed-off-by: Konrad Rzeszutek Wilk konrad.w...@oracle.com
---
src/libxl/libxl_domain.c| 19 +++
src/libxl/libxl_domain.h| 3 +++
src/libxl/libxl_driver.c| 6 ++
src/libxl/libxl_migration.c | 15 +--
4 files changed, 29 insertions(+), 14 deletions(-)
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index 5e0ab56..c038989 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -178,6 +178,25 @@ libxlDomainObjEndJob(libxlDriverPrivatePtr driver
ATTRIBUTE_UNUSED,
return virObjectUnref(obj);
}
+/*
+ * Checks whether the domain has host devices (PCIe) to disallow
+ * migration or saving of guest - unless they are detached.
+ *
+ * Returns true if the domain has host devices, otherwise false.
+ */
+bool
+libxlDomainHasHostDevices(virDomainDefPtr def)
+{
+/* Migration is not allowed if definition contains any hostdevs */
+if (def-nhostdevs 0) {
+virReportError(VIR_ERR_OPERATION_INVALID, %s,
+ _(domain has assigned host devices));
+return true;
+}
+
+return false;
+}
+
static void *
libxlDomainObjPrivateAlloc(void)
{
diff --git a/src/libxl/libxl_domain.h b/src/libxl/libxl_domain.h
index aa647b8..76c0c8d 100644
--- a/src/libxl/libxl_domain.h
+++ b/src/libxl/libxl_domain.h
@@ -95,6 +95,9 @@ char *
libxlDomainManagedSavePath(libxlDriverPrivatePtr driver,
virDomainObjPtr vm);
+bool
+libxlDomainHasHostDevices(virDomainDefPtr def);
+
int
libxlDomainSaveImageOpen(libxlDriverPrivatePtr driver,
libxlDriverConfigPtr cfg,
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 9eb071e..b9faba8 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -1650,6 +1650,9 @@ libxlDomainSaveFlags(virDomainPtr dom, const char *to,
const char *dxml,
goto endjob;
}
+if (libxlDomainHasHostDevices(vm-def))
+goto endjob;
+
if (libxlDoDomainSave(driver, vm, to) 0)
goto endjob;
@@ -1876,6 +1879,9 @@ libxlDomainManagedSave(virDomainPtr dom, unsigned int
flags)
goto endjob;
}
+if (libxlDomainHasHostDevices(vm-def))
+goto endjob;
+
name = libxlDomainManagedSavePath(driver, vm);
if (name == NULL)
goto endjob;
diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c
index 4010506..aaed448c 100644
--- a/src/libxl/libxl_migration.c
+++ b/src/libxl/libxl_migration.c
@@ -209,19 +209,6 @@ libxlDoMigrateSend(libxlDriverPrivatePtr driver,
return ret;
}
-static bool
-libxlDomainMigrationIsAllowed(virDomainDefPtr def)
-{
-/* Migration is not allowed if definition contains any hostdevs */
-if (def-nhostdevs 0) {
-virReportError(VIR_ERR_OPERATION_INVALID, %s,
- _(domain has assigned host devices));
-return false;
-}
-
-return true;
-}
-
char *
libxlDomainMigrationBegin(virConnectPtr conn,
virDomainObjPtr vm,
@@ -251,7 +238,7 @@ libxlDomainMigrationBegin(virConnectPtr conn,
def = vm-def;
}
-if (!libxlDomainMigrationIsAllowed(def))
+if (libxlDomainHasHostDevices(def))
goto endjob;
xml = virDomainDefFormat(def, VIR_DOMAIN_DEF_FORMAT_SECURE);
--
2.1.0
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
