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