Re: [libvirt] [PATCH] storage: forbid rebuilding existing disk storage pools

2011-11-14 Thread Guido Günther
On Mon, Nov 14, 2011 at 12:58:08PM +0800, Osier Yang wrote:
 于 2011年11月13日 00:19, Guido Günther 写道:
 which would blow away all volumes. Honor VIR_STORAGE_POOL_BUILD_OVERWRITE
 to force a rebuild.
 
 This was caught by libvirt-tck's storage/110-disk-pool.t.
 Cheers,
   -- Guido
 
 ---
   src/storage/storage_backend_disk.c |   72 
  ++--
   1 files changed, 68 insertions(+), 4 deletions(-)
 
 diff --git a/src/storage/storage_backend_disk.c 
 b/src/storage/storage_backend_disk.c
 index 82d6e8a..995ad2f 100644
 --- a/src/storage/storage_backend_disk.c
 +++ b/src/storage/storage_backend_disk.c
 @@ -335,6 +335,40 @@ virStorageBackendDiskRefreshPool(virConnectPtr conn 
 ATTRIBUTE_UNUSED,
 
 
   /**
 + * Check for a valid disk label (partition table) on device
 + *
 + * return: 0 - valid disk label found
 + *0 - no or unrecognized disk label
 + *0 - error finding the disk label
 + */
 +static int
 +virStorageBackendDiskFindLabel(const char* device)
 +{
 +const char *const args[] = {
 +device, print, --script, NULL,
 +};
 +virCommandPtr cmd = virCommandNew(PARTED);
 +char *output = NULL;
 +int ret = -1;
 +
 +virCommandAddArgSet(cmd, args);
 +virCommandAddEnvString(cmd, LC_ALL=C);
 +virCommandSetOutputBuffer(cmd,output);
 +
 +/* if parted succeeds we have a valid partition table */
 +ret = virCommandRun(cmd, NULL);
 +if (ret  0) {
 +if (strstr (output, unrecognised disk label))
 +ret = 1;
 +}
 +
 +virCommandFree(cmd);
 +VIR_FREE(output);
 +return ret;
 +}
 +
 +
 +/**
* Write a new partition table header
*/
   static int
 @@ -342,6 +376,8 @@ virStorageBackendDiskBuildPool(virConnectPtr conn 
 ATTRIBUTE_UNUSED,
  virStoragePoolObjPtr pool,
  unsigned int flags)
   {
 +bool ok_to_mklabel = false;
 +int ret = -1;
   /* eg parted /dev/sda mklabel msdos */
   const char *prog[] = {
   PARTED,
 @@ -353,12 +389,40 @@ virStorageBackendDiskBuildPool(virConnectPtr conn 
 ATTRIBUTE_UNUSED,
   NULL,
   };
 
 -virCheckFlags(0, -1);
 +virCheckFlags(VIR_STORAGE_POOL_BUILD_OVERWRITE |
 +  VIR_STORAGE_POOL_BUILD_NO_OVERWRITE, ret);
 
 -if (virRun(prog, NULL)  0)
 -return -1;
 +if (flags == (VIR_STORAGE_POOL_BUILD_OVERWRITE |
 +  VIR_STORAGE_POOL_BUILD_NO_OVERWRITE)) {
 +virStorageReportError(VIR_ERR_OPERATION_INVALID,
 +  _(Overwrite and no overwrite flags
 + are mutually exclusive));
 +goto error;
 +}
 
 -return 0;
 +if (flags  VIR_STORAGE_POOL_BUILD_OVERWRITE)
 +ok_to_mklabel = true;
 +else {
 +int check;
 +
 +check = virStorageBackendDiskFindLabel (
 +pool-def-source.devices[0].path);
 +if (check  0) {
 +ok_to_mklabel = true;
 +} else if (check  0) {
 +virStorageReportError(VIR_ERR_OPERATION_FAILED,
 +  _(Error checking for disk label));
 +} else {
 +virStorageReportError(VIR_ERR_OPERATION_INVALID,
 +  _(Disk label already present));
 +}
 +}
 +
 +if (ok_to_mklabel)
 +ret = virRun(prog, NULL);
 +
 +error:
 +return ret;
   }
 
   /**
 
 Looks fine, and having the flags is better, ACK
Pushed. Thanks.
 -- Guido

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

Re: [libvirt] [PATCH] storage: forbid rebuilding existing disk storage pools

2011-11-13 Thread Osier Yang

于 2011年11月13日 00:19, Guido Günther 写道:

which would blow away all volumes. Honor VIR_STORAGE_POOL_BUILD_OVERWRITE
to force a rebuild.

This was caught by libvirt-tck's storage/110-disk-pool.t.
Cheers,
  -- Guido

---
  src/storage/storage_backend_disk.c |   72 ++--
  1 files changed, 68 insertions(+), 4 deletions(-)

diff --git a/src/storage/storage_backend_disk.c 
b/src/storage/storage_backend_disk.c
index 82d6e8a..995ad2f 100644
--- a/src/storage/storage_backend_disk.c
+++ b/src/storage/storage_backend_disk.c
@@ -335,6 +335,40 @@ virStorageBackendDiskRefreshPool(virConnectPtr conn 
ATTRIBUTE_UNUSED,


  /**
+ * Check for a valid disk label (partition table) on device
+ *
+ * return: 0 - valid disk label found
+ *0 - no or unrecognized disk label
+ *0 - error finding the disk label
+ */
+static int
+virStorageBackendDiskFindLabel(const char* device)
+{
+const char *const args[] = {
+device, print, --script, NULL,
+};
+virCommandPtr cmd = virCommandNew(PARTED);
+char *output = NULL;
+int ret = -1;
+
+virCommandAddArgSet(cmd, args);
+virCommandAddEnvString(cmd, LC_ALL=C);
+virCommandSetOutputBuffer(cmd,output);
+
+/* if parted succeeds we have a valid partition table */
+ret = virCommandRun(cmd, NULL);
+if (ret  0) {
+if (strstr (output, unrecognised disk label))
+ret = 1;
+}
+
+virCommandFree(cmd);
+VIR_FREE(output);
+return ret;
+}
+
+
+/**
   * Write a new partition table header
   */
  static int
@@ -342,6 +376,8 @@ virStorageBackendDiskBuildPool(virConnectPtr conn 
ATTRIBUTE_UNUSED,
 virStoragePoolObjPtr pool,
 unsigned int flags)
  {
+bool ok_to_mklabel = false;
+int ret = -1;
  /* eg parted /dev/sda mklabel msdos */
  const char *prog[] = {
  PARTED,
@@ -353,12 +389,40 @@ virStorageBackendDiskBuildPool(virConnectPtr conn 
ATTRIBUTE_UNUSED,
  NULL,
  };

-virCheckFlags(0, -1);
+virCheckFlags(VIR_STORAGE_POOL_BUILD_OVERWRITE |
+  VIR_STORAGE_POOL_BUILD_NO_OVERWRITE, ret);

-if (virRun(prog, NULL)  0)
-return -1;
+if (flags == (VIR_STORAGE_POOL_BUILD_OVERWRITE |
+  VIR_STORAGE_POOL_BUILD_NO_OVERWRITE)) {
+virStorageReportError(VIR_ERR_OPERATION_INVALID,
+  _(Overwrite and no overwrite flags
+ are mutually exclusive));
+goto error;
+}

-return 0;
+if (flags  VIR_STORAGE_POOL_BUILD_OVERWRITE)
+ok_to_mklabel = true;
+else {
+int check;
+
+check = virStorageBackendDiskFindLabel (
+pool-def-source.devices[0].path);
+if (check  0) {
+ok_to_mklabel = true;
+} else if (check  0) {
+virStorageReportError(VIR_ERR_OPERATION_FAILED,
+  _(Error checking for disk label));
+} else {
+virStorageReportError(VIR_ERR_OPERATION_INVALID,
+  _(Disk label already present));
+}
+}
+
+if (ok_to_mklabel)
+ret = virRun(prog, NULL);
+
+error:
+return ret;
  }

  /**


Looks fine, and having the flags is better, ACK

Osier

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

[libvirt] [PATCH] storage: forbid rebuilding existing disk storage pools

2011-11-12 Thread Guido Günther
which would blow away all volumes. Honor VIR_STORAGE_POOL_BUILD_OVERWRITE
to force a rebuild.

This was caught by libvirt-tck's storage/110-disk-pool.t.
Cheers,
 -- Guido

---
 src/storage/storage_backend_disk.c |   72 ++--
 1 files changed, 68 insertions(+), 4 deletions(-)

diff --git a/src/storage/storage_backend_disk.c 
b/src/storage/storage_backend_disk.c
index 82d6e8a..995ad2f 100644
--- a/src/storage/storage_backend_disk.c
+++ b/src/storage/storage_backend_disk.c
@@ -335,6 +335,40 @@ virStorageBackendDiskRefreshPool(virConnectPtr conn 
ATTRIBUTE_UNUSED,
 
 
 /**
+ * Check for a valid disk label (partition table) on device
+ *
+ * return: 0 - valid disk label found
+ *0 - no or unrecognized disk label
+ *0 - error finding the disk label
+ */
+static int
+virStorageBackendDiskFindLabel(const char* device)
+{
+const char *const args[] = {
+device, print, --script, NULL,
+};
+virCommandPtr cmd = virCommandNew(PARTED);
+char *output = NULL;
+int ret = -1;
+
+virCommandAddArgSet(cmd, args);
+virCommandAddEnvString(cmd, LC_ALL=C);
+virCommandSetOutputBuffer(cmd, output);
+
+/* if parted succeeds we have a valid partition table */
+ret = virCommandRun(cmd, NULL);
+if (ret  0) {
+if (strstr (output, unrecognised disk label))
+ret = 1;
+}
+
+virCommandFree(cmd);
+VIR_FREE(output);
+return ret;
+}
+
+
+/**
  * Write a new partition table header
  */
 static int
@@ -342,6 +376,8 @@ virStorageBackendDiskBuildPool(virConnectPtr conn 
ATTRIBUTE_UNUSED,
virStoragePoolObjPtr pool,
unsigned int flags)
 {
+bool ok_to_mklabel = false;
+int ret = -1;
 /* eg parted /dev/sda mklabel msdos */
 const char *prog[] = {
 PARTED,
@@ -353,12 +389,40 @@ virStorageBackendDiskBuildPool(virConnectPtr conn 
ATTRIBUTE_UNUSED,
 NULL,
 };
 
-virCheckFlags(0, -1);
+virCheckFlags(VIR_STORAGE_POOL_BUILD_OVERWRITE |
+  VIR_STORAGE_POOL_BUILD_NO_OVERWRITE, ret);
 
-if (virRun(prog, NULL)  0)
-return -1;
+if (flags == (VIR_STORAGE_POOL_BUILD_OVERWRITE |
+  VIR_STORAGE_POOL_BUILD_NO_OVERWRITE)) {
+virStorageReportError(VIR_ERR_OPERATION_INVALID,
+  _(Overwrite and no overwrite flags
+ are mutually exclusive));
+goto error;
+}
 
-return 0;
+if (flags  VIR_STORAGE_POOL_BUILD_OVERWRITE)
+ok_to_mklabel = true;
+else {
+int check;
+
+check = virStorageBackendDiskFindLabel (
+pool-def-source.devices[0].path);
+if (check  0) {
+ok_to_mklabel = true;
+} else if (check  0) {
+virStorageReportError(VIR_ERR_OPERATION_FAILED,
+  _(Error checking for disk label));
+} else {
+virStorageReportError(VIR_ERR_OPERATION_INVALID,
+  _(Disk label already present));
+}
+}
+
+if (ok_to_mklabel)
+ret = virRun(prog, NULL);
+
+error:
+return ret;
 }
 
 /**
-- 
1.7.7.1

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