Re: [libvirt] [PATCH v5 9/9] virstoragefile: Add node-name

2015-05-12 Thread Peter Krempa
On Thu, Apr 23, 2015 at 14:41:21 +0200, Matthias Gatto wrote:
 Add nodename inside virstoragefile
 During xml backingStore parsing, look for a nodename attribute in the disk
 declaration if this one is a quorum, if a nodename is found, add it to
 the virStorageSource otherwise create a new one with a random name.
 Take inspiration from this patch to create the nodename:
 http://lists.gnu.org/archive/html/qemu-devel/2014-05/msg03209.html
 
 Durring xml backingStore formating, look for a nodename attribute inside the
 virStorageSource struct, and add it to the disk element.
 
 Use the nodename to create the quorum in qemuBuildQuorumStr.
 
 Signed-off-by: Matthias Gatto matthias.ga...@outscale.com
 ---

Once we decide that we want to deal with node names (which we definitely
should do soon we will need to take a different approach compared to
this patch:

  docs/formatdomain.html.in |  7 +++
  docs/schemas/domaincommon.rng |  5 +
  src/conf/domain_conf.c| 27 +++
  src/qemu/qemu_command.c   |  3 +++
  src/util/virstoragefile.c |  4 
  src/util/virstoragefile.h |  1 +
  6 files changed, 47 insertions(+)
 
 diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
 index 7d058ec..d9afe36 100644
 --- a/docs/formatdomain.html.in
 +++ b/docs/formatdomain.html.in
 @@ -2183,6 +2183,13 @@
  codevda[2]/code refers to the backing store with
  codeindex='2'/code of the disk with codevda/code target.
/dd
 +  dtcodenodename/code attribute
 +  span class=sincesince 1.2.13/span/dt
 +  dd
 +When the backing store is a quorum child, we can use this 
 attribute
 +to define the node-name of a child. If this atribute is undefine,
 +a random nodename is generate.

We certainly don't want to give the user the need to specify node names.
In fact I think libvirt shouldn't expose node names in any way. The
implementation should remain internal and users will interact via the
backing chain 'index' element:

'This attribute is only valid in output (and ignored on input) and it
can be used to refer to a specific part of the disk chain when doing
block operations (such as via the virDomainBlockRebase API). For
example, vda[2] refers to the backing store with index='2' of the disk
with vda target.'

Once we do this we should specify a node name  for every backing chain
element or possibly re-detect it after qemu starts and store the
backing chain info internally. This will be necessary as libvirt has to
model the operations with the backing chain the same way as qemu is
doing it so that libvirt can ensure that qemu is not accessing files
that it should not access.

At any rate, node names are a very useful concept, but this patch would
be a step in the wrong direction.

Peter


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v5 9/9] virstoragefile: Add node-name

2015-04-23 Thread Matthias Gatto
Add nodename inside virstoragefile
During xml backingStore parsing, look for a nodename attribute in the disk
declaration if this one is a quorum, if a nodename is found, add it to
the virStorageSource otherwise create a new one with a random name.
Take inspiration from this patch to create the nodename:
http://lists.gnu.org/archive/html/qemu-devel/2014-05/msg03209.html

Durring xml backingStore formating, look for a nodename attribute inside the
virStorageSource struct, and add it to the disk element.

Use the nodename to create the quorum in qemuBuildQuorumStr.

Signed-off-by: Matthias Gatto matthias.ga...@outscale.com
---
 docs/formatdomain.html.in |  7 +++
 docs/schemas/domaincommon.rng |  5 +
 src/conf/domain_conf.c| 27 +++
 src/qemu/qemu_command.c   |  3 +++
 src/util/virstoragefile.c |  4 
 src/util/virstoragefile.h |  1 +
 6 files changed, 47 insertions(+)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 7d058ec..d9afe36 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2183,6 +2183,13 @@
 codevda[2]/code refers to the backing store with
 codeindex='2'/code of the disk with codevda/code target.
   /dd
+  dtcodenodename/code attribute
+  span class=sincesince 1.2.13/span/dt
+  dd
+When the backing store is a quorum child, we can use this attribute
+to define the node-name of a child. If this atribute is undefine,
+a random nodename is generate.
+  /dd
   dtcodeformat/code sub-element/dt
   dd
 The codeformat/code element contains codetype/code
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 6cd834e..30694d9 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1236,6 +1236,11 @@
 ref name=positiveInteger/
   /attribute
   interleave
+optional
+  attribute name=nodename
+text/
+  /attribute
+/optional
 ref name=diskSource/
 ref name=diskBackingChain/
 ref name=diskFormat/
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index ec93b6f..46eeae2 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -54,6 +54,7 @@
 #include network_conf.h
 #include virtpm.h
 #include virstring.h
+#include virrandom.h
 
 #define VIR_FROM_THIS VIR_FROM_DOMAIN
 
@@ -5978,6 +5979,8 @@ virDomainDiskThresholdParse(virStorageSourcePtr src,
 }
 
 
+#define GEN_NODE_NAME_PREFIXlibvirt
+#define GEN_NODE_NAME_MAX_LEN   (sizeof(GEN_NODE_NAME_PREFIX) + 8 + 8)
 static int
 virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt,
virStorageSourcePtr src,
@@ -6020,6 +6023,26 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt,
 goto cleanup;
 }
 
+if (src-type == VIR_STORAGE_TYPE_QUORUM) {
+char *nodeName = NULL;
+
+if ((nodeName = virXMLPropString(ctxt-node, nodename))) {
+backingStore-nodeName = nodeName;
+} else {
+size_t len;
+
+if (VIR_ALLOC_N(nodeName, GEN_NODE_NAME_MAX_LEN)  0)
+goto cleanup;
+if (snprintf(nodeName, GEN_NODE_NAME_MAX_LEN,
+ %s%08x, GEN_NODE_NAME_PREFIX, (int)pos)  0)
+goto cleanup;
+for (len = strlen(nodeName); len  GEN_NODE_NAME_MAX_LEN - 1; 
++len)
+nodeName[len] = virRandomInt('Z' - 'A') + 'A';
+nodeName[GEN_NODE_NAME_MAX_LEN - 1] = '\0';
+backingStore-nodeName = nodeName;
+}
+}
+
 if (!(format = virXPathString(string(./format/@type), ctxt))) {
 virReportError(VIR_ERR_XML_ERROR, %s,
_(missing disk backing store format));
@@ -6075,6 +6098,8 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt,
 ctxt-node = save_ctxt;
 return ret;
 }
+#undef GEN_NODE_NAME_PREFIX
+#undef GEN_NODE_NAME_MAX_LEN
 
 #define VENDOR_LEN  8
 #define PRODUCT_LEN 16
@@ -17817,6 +17842,8 @@ virDomainDiskBackingStoreFormat(virBufferPtr buf,
   type, idx);
 if (backingStore-threshold)
 virBufferAsprintf(buf,  threshold='%lu', 
backingStore-threshold);
+if (backingStore-nodeName)
+virBufferAsprintf(buf,  nodename='%s', backingStore-nodeName);
 virBufferAddLit(buf, \n);
 virBufferAdjustIndent(buf, 2);
 
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 80cbb7d..ac09750 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3567,6 +3567,9 @@ qemuBuildQuorumStr(virConnectPtr conn,
   toAppend, i,
   
virStorageFileFormatTypeToString(backingStore-format));
 
+virBufferAsprintf(opt, ,%schildren.%lu.node-name=%s,
+  toAppend, i, backingStore-nodeName);
+