Re: [libvirt] [PATCH v5 8/9] qemu: Add quorum support in qemuBuildDriveDevStr

2015-06-04 Thread Matthias Gatto
On Tue, May 12, 2015 at 5:38 PM, Peter Krempa pkre...@redhat.com wrote:
 On Thu, Apr 23, 2015 at 14:41:20 +0200, Matthias Gatto wrote:
 Allow to libvirt to build the quorum string used by quemu.

 Add 2 static functions: qemuBuildQuorumStr and
 qemuBuildAndAppendDriveStrToVirBuffer.
 qemuBuildQuorumStr is made because a quorum can have another quorum
 as a child, so we may need to call qemuBuildQuorumStr recursively.

 qemuBuildQuorumFileSourceStr was basically made to share
 the code use to build the source between qemuBuildQuorumStr and
 qemuBuildDriveStr, but there is some difference betwin the syntax
 use by libvirt to declare a disk and the one qemu need to build a quorum:
 a quorum need a syntaxe like:
 domaine_name.children.X.file.filename=filename
 where libvirt don't use file.filename= but directly file=.
 Therfore I use this function only for quorum.

 But as explained in the cover letter and here:
 http://www.redhat.com/archives/libvir-list/2014-October/msg00529.html
 We miss some informations in _virStorageSource to have a complet
 quorum support in libvirt.
 Ideally I'd like to refactore virDomainDiskDefFormat to allow
 qemuBuildQuorumStr to call this function in a loop.

 Signed-off-by: Matthias Gatto matthias.ga...@outscale.com
 ---
  src/qemu/qemu_command.c | 110 
 
  1 file changed, 110 insertions(+)


 ...

 diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
 index 6c40d3e..80cbb7d 100644
 --- a/src/qemu/qemu_command.c
 +++ b/src/qemu/qemu_command.c
 @@ -3479,6 +3479,111 @@ qemuCheckDiskConfig(virDomainDiskDefPtr disk)
  return -1;
  }

 +static bool
 +qemuBuildQuorumFileSourceStr(virConnectPtr conn,
 +  virStorageSourcePtr src,
 +  virBuffer *opt,
 +  const char *toAppend)
 +{
 +char *source = NULL;
 +int actualType = virStorageSourceGetActualType(src);
 +
 +if (qemuGetDriveSourceString(src, conn, source)  0)
 +goto error;
 +
 +if (source) {
 +
 +virBufferAsprintf(opt, ,%sfilename=, toAppend);

 virBufferStrcat

 +
 +switch (actualType) {
 +case VIR_STORAGE_TYPE_DIR:

 I'd forbid the DIR type altogether with quorums.

 +/* QEMU only supports magic FAT format for now */
 +if (src-format  0 
 +src-format != VIR_STORAGE_FILE_FAT) {
 +virReportError(VIR_ERR_INTERNAL_ERROR,
 +   _(unsupported disk driver type for '%s'),
 +   
 virStorageFileFormatTypeToString(src-format));
 +goto error;
 +}
 +
 +if (!src-readonly) {
 +virReportError(VIR_ERR_INTERNAL_ERROR, %s,
 +   _(cannot create virtual FAT disks in 
 read-write mode));
 +goto error;
 +}
 +
 +virBufferAddLit(opt, fat:);
 +
 +break;
 +
 +default:
 +break;
 +}
 +virBufferAsprintf(opt, %s, source);

 virBufferAdd

 +}
 +
 +return true;
 + error:
 +return false;

 Error can be returned right away since there is nothing to clean up.

 +}
 +
 +
 +static bool
 +qemuBuildQuorumStr(virConnectPtr conn,
 +   virDomainDiskDefPtr disk,
 +   virStorageSourcePtr src,
 +   virBuffer *opt,
 +   const char *toAppend)
 +{
 +char *tmp = NULL;
 +int ret;
 +virStorageSourcePtr backingStore;
 +size_t i;
 +
 +if (!src-threshold) {
 +virReportError(VIR_ERR_INTERNAL_ERROR, %s,
 +   _(threshold missing in the quorum configuration));
 +return false;
 +}
 +if (src-nBackingStores  2) {
 +virReportError(VIR_ERR_INTERNAL_ERROR, %s,
 +   _(a quorum must have at last 2 children));
 +return false;
 +}
 +if (src-threshold  src-nBackingStores) {
 +virReportError(VIR_ERR_INTERNAL_ERROR, %s,
 +   _(threshold must not exceed the number of 
 childrens));

 'children' is the proper plural

 +return false;
 +}
 +virBufferAsprintf(opt, ,%svote-threshold=%lu,
 +  toAppend, src-threshold);
 +for (i = 0;  i  src-nBackingStores; ++i) {
 +backingStore = virStorageSourceGetBackingStore(src, i);
 +ret = virAsprintf(tmp, %schildren.%lu.file., toAppend, i);
 +if (ret  0)
 +return false;
 +
 +virBufferAsprintf(opt, ,%schildren.%lu.driver=%s,
 +  toAppend, i,
 +  
 virStorageFileFormatTypeToString(backingStore-format));
 +
 +if (qemuBuildQuorumFileSourceStr(conn, backingStore, opt, tmp) == 
 false)
 +goto error;
 +
 +/* This operation avoid me to made another copy */
 +tmp[ret - sizeof(file)] = '\0';
 +if 

Re: [libvirt] [PATCH v5 8/9] qemu: Add quorum support in qemuBuildDriveDevStr

2015-05-12 Thread Peter Krempa
On Thu, Apr 23, 2015 at 14:41:20 +0200, Matthias Gatto wrote:
 Allow to libvirt to build the quorum string used by quemu.
 
 Add 2 static functions: qemuBuildQuorumStr and
 qemuBuildAndAppendDriveStrToVirBuffer.
 qemuBuildQuorumStr is made because a quorum can have another quorum
 as a child, so we may need to call qemuBuildQuorumStr recursively.
 
 qemuBuildQuorumFileSourceStr was basically made to share
 the code use to build the source between qemuBuildQuorumStr and
 qemuBuildDriveStr, but there is some difference betwin the syntax
 use by libvirt to declare a disk and the one qemu need to build a quorum:
 a quorum need a syntaxe like:
 domaine_name.children.X.file.filename=filename
 where libvirt don't use file.filename= but directly file=.
 Therfore I use this function only for quorum.
 
 But as explained in the cover letter and here:
 http://www.redhat.com/archives/libvir-list/2014-October/msg00529.html
 We miss some informations in _virStorageSource to have a complet
 quorum support in libvirt.
 Ideally I'd like to refactore virDomainDiskDefFormat to allow
 qemuBuildQuorumStr to call this function in a loop.
 
 Signed-off-by: Matthias Gatto matthias.ga...@outscale.com
 ---
  src/qemu/qemu_command.c | 110 
 
  1 file changed, 110 insertions(+)
 

...

 diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
 index 6c40d3e..80cbb7d 100644
 --- a/src/qemu/qemu_command.c
 +++ b/src/qemu/qemu_command.c
 @@ -3479,6 +3479,111 @@ qemuCheckDiskConfig(virDomainDiskDefPtr disk)
  return -1;
  }
  
 +static bool
 +qemuBuildQuorumFileSourceStr(virConnectPtr conn,
 +  virStorageSourcePtr src,
 +  virBuffer *opt,
 +  const char *toAppend)
 +{
 +char *source = NULL;
 +int actualType = virStorageSourceGetActualType(src);
 +
 +if (qemuGetDriveSourceString(src, conn, source)  0)
 +goto error;
 +
 +if (source) {
 +
 +virBufferAsprintf(opt, ,%sfilename=, toAppend);

virBufferStrcat

 +
 +switch (actualType) {
 +case VIR_STORAGE_TYPE_DIR:

I'd forbid the DIR type altogether with quorums.

 +/* QEMU only supports magic FAT format for now */
 +if (src-format  0 
 +src-format != VIR_STORAGE_FILE_FAT) {
 +virReportError(VIR_ERR_INTERNAL_ERROR,
 +   _(unsupported disk driver type for '%s'),
 +   
 virStorageFileFormatTypeToString(src-format));
 +goto error;
 +}
 +
 +if (!src-readonly) {
 +virReportError(VIR_ERR_INTERNAL_ERROR, %s,
 +   _(cannot create virtual FAT disks in 
 read-write mode));
 +goto error;
 +}
 +
 +virBufferAddLit(opt, fat:);
 +
 +break;
 +
 +default:
 +break;
 +}
 +virBufferAsprintf(opt, %s, source);

virBufferAdd

 +}
 +
 +return true;
 + error:
 +return false;

Error can be returned right away since there is nothing to clean up.

 +}
 +
 +
 +static bool
 +qemuBuildQuorumStr(virConnectPtr conn,
 +   virDomainDiskDefPtr disk,
 +   virStorageSourcePtr src,
 +   virBuffer *opt,
 +   const char *toAppend)
 +{
 +char *tmp = NULL;
 +int ret;
 +virStorageSourcePtr backingStore;
 +size_t i;
 +
 +if (!src-threshold) {
 +virReportError(VIR_ERR_INTERNAL_ERROR, %s,
 +   _(threshold missing in the quorum configuration));
 +return false;
 +}
 +if (src-nBackingStores  2) {
 +virReportError(VIR_ERR_INTERNAL_ERROR, %s,
 +   _(a quorum must have at last 2 children));
 +return false;
 +}
 +if (src-threshold  src-nBackingStores) {
 +virReportError(VIR_ERR_INTERNAL_ERROR, %s,
 +   _(threshold must not exceed the number of 
 childrens));

'children' is the proper plural

 +return false;
 +}
 +virBufferAsprintf(opt, ,%svote-threshold=%lu,
 +  toAppend, src-threshold);
 +for (i = 0;  i  src-nBackingStores; ++i) {
 +backingStore = virStorageSourceGetBackingStore(src, i);
 +ret = virAsprintf(tmp, %schildren.%lu.file., toAppend, i);
 +if (ret  0)
 +return false;
 +
 +virBufferAsprintf(opt, ,%schildren.%lu.driver=%s,
 +  toAppend, i,
 +  
 virStorageFileFormatTypeToString(backingStore-format));
 +
 +if (qemuBuildQuorumFileSourceStr(conn, backingStore, opt, tmp) == 
 false)
 +goto error;
 +
 +/* This operation avoid me to made another copy */
 +tmp[ret - sizeof(file)] = '\0';
 +if (backingStore-type == VIR_STORAGE_TYPE_QUORUM) {
 +if 

[libvirt] [PATCH v5 8/9] qemu: Add quorum support in qemuBuildDriveDevStr

2015-04-23 Thread Matthias Gatto
Allow to libvirt to build the quorum string used by quemu.

Add 2 static functions: qemuBuildQuorumStr and
qemuBuildAndAppendDriveStrToVirBuffer.
qemuBuildQuorumStr is made because a quorum can have another quorum
as a child, so we may need to call qemuBuildQuorumStr recursively.

qemuBuildQuorumFileSourceStr was basically made to share
the code use to build the source between qemuBuildQuorumStr and
qemuBuildDriveStr, but there is some difference betwin the syntax
use by libvirt to declare a disk and the one qemu need to build a quorum:
a quorum need a syntaxe like:
domaine_name.children.X.file.filename=filename
where libvirt don't use file.filename= but directly file=.
Therfore I use this function only for quorum.

But as explained in the cover letter and here:
http://www.redhat.com/archives/libvir-list/2014-October/msg00529.html
We miss some informations in _virStorageSource to have a complet
quorum support in libvirt.
Ideally I'd like to refactore virDomainDiskDefFormat to allow
qemuBuildQuorumStr to call this function in a loop.

Signed-off-by: Matthias Gatto matthias.ga...@outscale.com
---
 src/qemu/qemu_command.c | 110 
 1 file changed, 110 insertions(+)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 6c40d3e..80cbb7d 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3479,6 +3479,111 @@ qemuCheckDiskConfig(virDomainDiskDefPtr disk)
 return -1;
 }
 
+static bool
+qemuBuildQuorumFileSourceStr(virConnectPtr conn,
+  virStorageSourcePtr src,
+  virBuffer *opt,
+  const char *toAppend)
+{
+char *source = NULL;
+int actualType = virStorageSourceGetActualType(src);
+
+if (qemuGetDriveSourceString(src, conn, source)  0)
+goto error;
+
+if (source) {
+
+virBufferAsprintf(opt, ,%sfilename=, toAppend);
+
+switch (actualType) {
+case VIR_STORAGE_TYPE_DIR:
+/* QEMU only supports magic FAT format for now */
+if (src-format  0 
+src-format != VIR_STORAGE_FILE_FAT) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(unsupported disk driver type for '%s'),
+   virStorageFileFormatTypeToString(src-format));
+goto error;
+}
+
+if (!src-readonly) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(cannot create virtual FAT disks in 
read-write mode));
+goto error;
+}
+
+virBufferAddLit(opt, fat:);
+
+break;
+
+default:
+break;
+}
+virBufferAsprintf(opt, %s, source);
+}
+
+return true;
+ error:
+return false;
+}
+
+
+static bool
+qemuBuildQuorumStr(virConnectPtr conn,
+   virDomainDiskDefPtr disk,
+   virStorageSourcePtr src,
+   virBuffer *opt,
+   const char *toAppend)
+{
+char *tmp = NULL;
+int ret;
+virStorageSourcePtr backingStore;
+size_t i;
+
+if (!src-threshold) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(threshold missing in the quorum configuration));
+return false;
+}
+if (src-nBackingStores  2) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(a quorum must have at last 2 children));
+return false;
+}
+if (src-threshold  src-nBackingStores) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(threshold must not exceed the number of childrens));
+return false;
+}
+virBufferAsprintf(opt, ,%svote-threshold=%lu,
+  toAppend, src-threshold);
+for (i = 0;  i  src-nBackingStores; ++i) {
+backingStore = virStorageSourceGetBackingStore(src, i);
+ret = virAsprintf(tmp, %schildren.%lu.file., toAppend, i);
+if (ret  0)
+return false;
+
+virBufferAsprintf(opt, ,%schildren.%lu.driver=%s,
+  toAppend, i,
+  
virStorageFileFormatTypeToString(backingStore-format));
+
+if (qemuBuildQuorumFileSourceStr(conn, backingStore, opt, tmp) == 
false)
+goto error;
+
+/* This operation avoid me to made another copy */
+tmp[ret - sizeof(file)] = '\0';
+if (backingStore-type == VIR_STORAGE_TYPE_QUORUM) {
+if (!qemuBuildQuorumStr(conn, disk, backingStore, opt, tmp))
+goto error;
+}
+VIR_FREE(tmp);
+}
+return true;
+ error:
+VIR_FREE(tmp);
+return false;
+}
+
 
 /* Qemu 1.2 and later have a binary flag -enable-fips that must be
  * used for VNC auth to obey FIPS settings; but the flag only
@@ -3952,6 +4057,11 @@ qemuBuildDriveStr(virConnectPtr conn,