Re: [libvirt] [PATCH] libxl: Disallow save or migrate when host devices are assigned to a guest.

2015-04-15 Thread Konrad Rzeszutek Wilk 
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.

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

Re: [libvirt] [PATCH] libxl: Disallow save or migrate when host devices are assigned to a guest.

2015-04-14 Thread Jim Fehlig 
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

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

[libvirt] [PATCH] libxl: Disallow save or migrate when host devices are assigned to a guest.

2015-04-08 Thread Konrad Rzeszutek Wilk 
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

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