11:13 p.m.
The current support for qemu and Ceph RBD (rados block device) has two
main deficiencies: authentication doesn't work, and it relies on
environment variables (which don't work with latest upstream). This
patch set addresses both those problems.
The first two patches update the xml schemas and conf to add a Ceph
secret type and to specify authentication information along with the rbd
disk.
The next two patches make some libvirt changes. We pass virConnectPtr
down into the Domain{Attach,Detach} methods (needed to access secrets
while building the qemu command), and add a helper that will escape
arbitrary characters.
The final patch replaces the current RBD qemu code and uses the new conf
info to do authentication properly. (We still need to make a change
there to avoid having the authentication key show up on qemu command
line; I'll clean that up shortly.)
Comments on this approach?
Thanks!
sage
Changes from v1:
update docs/schemas/{domain,secret}.rng
Sage Weil (5):
secret: add Ceph secret type
storage: add authId, authDomain to virDomainDiskDef
qemu: pass virConnectPtr into Domain{Attach,Detach}*
buf: implement generic virBufferEscape
qemu/rbd: improve rbd device specification
docs/schemas/domain.rng | 6 +
docs/schemas/secret.rng | 17 ++
include/libvirt/libvirt.h.in | 3 +
src/conf/domain_conf.c | 43 +++-
src/conf/domain_conf.h | 2 +
src/conf/secret_conf.c | 45 ++++-
src/conf/secret_conf.h | 1 +
src/libvirt_private.syms | 1 +
src/qemu/qemu_command.c | 273 +++++++++++---------
src/qemu/qemu_command.h | 3 +-
src/qemu/qemu_driver.c | 17 +-
src/qemu/qemu_hotplug.c | 15 +-
src/qemu/qemu_hotplug.h | 9 +-
src/secret/secret_driver.c | 8 +
src/util/buf.c | 33 ++-
src/util/buf.h | 1 +
.../qemuxml2argv-disk-drive-network-rbd.args | 6 +-
.../qemuxml2argv-disk-drive-network-rbd.xml | 1 +
18 files changed, 328 insertions(+), 156 deletions(-)
--
1.7.4.1
From 498cd06b76bbb4415a2f81f9d169f267ff99329c Mon Sep 17 00:00:00 2001
From: Sage Weil <sage(a)newdream.net>
Date: Thu, 15 Sep 2011 13:47:40 -0700
Subject: [PATCH 0/5] Improve Ceph Qemu+RBD support
The current support for qemu and Ceph RBD (rados block device) has two
main deficiencies: authentication doesn't work, and it relies on
environment variables (which don't work with latest upstream).
This patch set addresses both those problems, while trying to integrate as
cleanly as possible with the rest of libvirt.
The first few patches make some changes to libvirt itself: adding a CEPH
secret type (for Ceph/RBD authentication), adding authentication fields
to the XML schema, passing the virConnectPtr into the
Domain{Attach,Detach} methods (needed to access secrets while building
the qemu command), a helper that will escape arbitrary characters, and
finally a patch that replaces the current RBD qemu code.
Comments on this approach?
Thanks!
sage
Sage Weil (5):
secret: add Ceph secret type
storage: add authId, authDomain to virDomainDiskDef
qemu: pass virConnectPtr into Domain{Attach,Detach}*
buf: implement generic virBufferEscape
qemu/rbd: improve rbd device specification
include/libvirt/libvirt.h.in | 3 +
src/conf/domain_conf.c | 43 +++-
src/conf/domain_conf.h | 2 +
src/conf/secret_conf.c | 45 ++++-
src/conf/secret_conf.h | 1 +
src/libvirt_private.syms | 1 +
src/qemu/qemu_command.c | 273 +++++++++++---------
src/qemu/qemu_command.h | 3 +-
src/qemu/qemu_driver.c | 17 +-
src/qemu/qemu_hotplug.c | 15 +-
src/qemu/qemu_hotplug.h | 9 +-
src/secret/secret_driver.c | 8 +
src/util/buf.c | 33 ++-
src/util/buf.h | 1 +
.../qemuxml2argv-disk-drive-network-rbd.args | 6 +-
.../qemuxml2argv-disk-drive-network-rbd.xml | 1 +
16 files changed, 305 insertions(+), 156 deletions(-)
--
1.7.4.1
11:13 p.m.
New subject: [libvirt] [PATCH 1/5] secret: add Ceph secret type
Add a new secret type to store a Ceph authentication key. The ceph_id
field contains the name of the key (e.g. 'admin' for the ceph superuser).
Signed-off-by: Sage Weil <sage(a)newdream.net>
---
docs/schemas/secret.rng | 17 +++++++++++++++
include/libvirt/libvirt.h.in | 3 ++
src/conf/secret_conf.c | 45 +++++++++++++++++++++++++++++++++++++++++-
src/conf/secret_conf.h | 1 +
src/secret/secret_driver.c | 8 +++++++
5 files changed, 73 insertions(+), 1 deletions(-)
diff --git a/docs/schemas/secret.rng b/docs/schemas/secret.rng
index 80270ae..c3da8b3 100644
--- a/docs/schemas/secret.rng
+++ b/docs/schemas/secret.rng
@@ -37,6 +37,7 @@
<element name='usage'>
<choice>
<ref name='usagevolume'/>
+ <ref name='cephauth'/>
<!-- More choices later -->
</choice>
</element>
@@ -54,6 +55,22 @@
</element>
</define>
+ <define name='cephauth'>
+ <attribute name='type'>
+ <value>ceph</value>
+ </attribute>
+ <element name='auth'>
+ <attribute name='id'>
+ <text/>
+ </attribute>
+ <optional>
+ <attribute name='domain'>
+ <text/>
+ </attribute>
+ </optional>
+ </element>
+ </define>
+
<define name="UUID">
<choice>
<data type="string">
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index b1bda31..51fd044 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -2257,7 +2257,10 @@ typedef virSecret *virSecretPtr;
typedef enum {
VIR_SECRET_USAGE_TYPE_NONE = 0,
VIR_SECRET_USAGE_TYPE_VOLUME = 1,
+ VIR_SECRET_USAGE_TYPE_CEPH = 2,
/* Expect more owner types later... */
+
+ VIR_SECRET_USAGE_TYPE_LAST
} virSecretUsageType;
virConnectPtr virSecretGetConnect (virSecretPtr secret);
diff --git a/src/conf/secret_conf.c b/src/conf/secret_conf.c
index 105afbe..8f11a51 100644
--- a/src/conf/secret_conf.c
+++ b/src/conf/secret_conf.c
@@ -35,7 +35,8 @@
#define VIR_FROM_THIS VIR_FROM_SECRET
-VIR_ENUM_IMPL(virSecretUsageType, VIR_SECRET_USAGE_TYPE_VOLUME + 1, "none",
"volume")
+VIR_ENUM_IMPL(virSecretUsageType, VIR_SECRET_USAGE_TYPE_LAST,
+ "none", "volume", "ceph")
void
virSecretDefFree(virSecretDefPtr def)
@@ -52,6 +53,10 @@ virSecretDefFree(virSecretDefPtr def)
VIR_FREE(def->usage.volume);
break;
+ case VIR_SECRET_USAGE_TYPE_CEPH:
+ VIR_FREE(def->usage.authIdDomain);
+ break;
+
default:
VIR_ERROR(_("unexpected secret usage type %d"), def->usage_type);
break;
@@ -65,6 +70,8 @@ virSecretDefParseUsage(xmlXPathContextPtr ctxt,
{
char *type_str;
int type;
+ char *authId, *authDomain;
+ int ret;
type_str = virXPathString("string(./usage/@type)", ctxt);
if (type_str == NULL) {
@@ -94,6 +101,27 @@ virSecretDefParseUsage(xmlXPathContextPtr ctxt,
}
break;
+ case VIR_SECRET_USAGE_TYPE_CEPH:
+ authId = virXPathString("string(./usage/auth/@id)", ctxt);
+ if (!authId) {
+ virSecretReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("ceph usage specified, but auth id is
missing"));
+ return -1;
+ }
+ authDomain = virXPathString("string(./usage/auth/@domain)", ctxt);
+ if (!authDomain) {
+ VIR_FREE(authId);
+ virSecretReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("ceph usage specified, but auth domain is
missing"));
+ return -1;
+ }
+ ret = virAlloc(&def->usage.authIdDomain, strlen(authId) +
+ strlen(authDomain) + 2);
+ sprintf(def->usage.authIdDomain, "%s/%s", authId, authDomain);
+ VIR_FREE(authId);
+ VIR_FREE(authDomain);
+ break;
+
default:
virSecretReportError(VIR_ERR_INTERNAL_ERROR,
_("unexpected secret usage type %d"),
@@ -220,6 +248,9 @@ virSecretDefFormatUsage(virBufferPtr buf,
const virSecretDefPtr def)
{
const char *type;
+ char *p;
+ char idAuth[80];
+ int len;
type = virSecretUsageTypeTypeToString(def->usage_type);
if (type == NULL) {
@@ -239,6 +270,18 @@ virSecretDefFormatUsage(virBufferPtr buf,
def->usage.volume);
break;
+ case VIR_SECRET_USAGE_TYPE_CEPH:
+ if (def->usage.authIdDomain != NULL) {
+ p = strchr(def->usage.authIdDomain, '/');
+ len = p - def->usage.authIdDomain;
+ strncpy(idAuth, def->usage.authIdDomain, len);
+ idAuth[len] = '\0';
+ p++;
+ virBufferEscapeString(buf, " <auth id='%s'",
idAuth);
+ virBufferEscapeString(buf, " domain='%s'/>\n", p);
+ }
+ break;
+
default:
virSecretReportError(VIR_ERR_INTERNAL_ERROR,
_("unexpected secret usage type %d"),
diff --git a/src/conf/secret_conf.h b/src/conf/secret_conf.h
index 4b47c52..294e7ab 100644
--- a/src/conf/secret_conf.h
+++ b/src/conf/secret_conf.h
@@ -42,6 +42,7 @@ struct _virSecretDef {
int usage_type;
union {
char *volume; /* May be NULL */
+ char *authIdDomain;
} usage;
};
diff --git a/src/secret/secret_driver.c b/src/secret/secret_driver.c
index 59dc687..7ea8a49 100644
--- a/src/secret/secret_driver.c
+++ b/src/secret/secret_driver.c
@@ -144,6 +144,11 @@ secretFindByUsage(virSecretDriverStatePtr driver, int usageType,
const char *usa
if (STREQ(s->def->usage.volume, usageID))
return s;
break;
+
+ case VIR_SECRET_USAGE_TYPE_CEPH:
+ if (STREQ(s->def->usage.authIdDomain, usageID))
+ return s;
+ break;
}
}
return NULL;
@@ -607,6 +612,9 @@ secretUsageIDForDef(virSecretDefPtr def)
case VIR_SECRET_USAGE_TYPE_VOLUME:
return def->usage.volume;
+ case VIR_SECRET_USAGE_TYPE_CEPH:
+ return def->usage.authIdDomain;
+
default:
return NULL;
}
--
1.7.4.1
11:36 a.m.
New subject: [libvirt] [PATCH 1/5] secret: add Ceph secret type
On Mon, Sep 19, 2011 at 09:13:39PM -0700, Sage Weil wrote:
Add a new secret type to store a Ceph authentication key. The
ceph_id
field contains the name of the key (e.g. 'admin' for the ceph superuser).
Signed-off-by: Sage Weil <sage(a)newdream.net>
---
docs/schemas/secret.rng | 17 +++++++++++++++
include/libvirt/libvirt.h.in | 3 ++
src/conf/secret_conf.c | 45 +++++++++++++++++++++++++++++++++++++++++-
src/conf/secret_conf.h | 1 +
src/secret/secret_driver.c | 8 +++++++
5 files changed, 73 insertions(+), 1 deletions(-)
diff --git a/docs/schemas/secret.rng b/docs/schemas/secret.rng
index 80270ae..c3da8b3 100644
--- a/docs/schemas/secret.rng
+++ b/docs/schemas/secret.rng
@@ -37,6 +37,7 @@
<element name='usage'>
<choice>
<ref name='usagevolume'/>
+ <ref name='cephauth'/>
<!-- More choices later -->
</choice>
</element>
@@ -54,6 +55,22 @@
</element>
</define>
+ <define name='cephauth'>
+ <attribute name='type'>
+ <value>ceph</value>
+ </attribute>
+ <element name='auth'>
+ <attribute name='id'>
+ <text/>
+ </attribute>
+ <optional>
+ <attribute name='domain'>
+ <text/>
+ </attribute>
Here I would expect just
<element name='domain'>
<text/>
</element>
+ </optional>
+ </element>
+ </define>
+
<define name="UUID">
<choice>
<data type="string">
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index b1bda31..51fd044 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -2257,7 +2257,10 @@ typedef virSecret *virSecretPtr;
typedef enum {
VIR_SECRET_USAGE_TYPE_NONE = 0,
VIR_SECRET_USAGE_TYPE_VOLUME = 1,
+ VIR_SECRET_USAGE_TYPE_CEPH = 2,
/* Expect more owner types later... */
+
+ VIR_SECRET_USAGE_TYPE_LAST
} virSecretUsageType;
virConnectPtr virSecretGetConnect (virSecretPtr secret);
diff --git a/src/conf/secret_conf.c b/src/conf/secret_conf.c
index 105afbe..8f11a51 100644
--- a/src/conf/secret_conf.c
+++ b/src/conf/secret_conf.c
@@ -35,7 +35,8 @@
#define VIR_FROM_THIS VIR_FROM_SECRET
-VIR_ENUM_IMPL(virSecretUsageType, VIR_SECRET_USAGE_TYPE_VOLUME + 1, "none",
"volume")
+VIR_ENUM_IMPL(virSecretUsageType, VIR_SECRET_USAGE_TYPE_LAST,
+ "none", "volume", "ceph")
void
virSecretDefFree(virSecretDefPtr def)
@@ -52,6 +53,10 @@ virSecretDefFree(virSecretDefPtr def)
VIR_FREE(def->usage.volume);
break;
+ case VIR_SECRET_USAGE_TYPE_CEPH:
+ VIR_FREE(def->usage.authIdDomain);
+ break;
+
default:
VIR_ERROR(_("unexpected secret usage type %d"), def->usage_type);
break;
@@ -65,6 +70,8 @@ virSecretDefParseUsage(xmlXPathContextPtr ctxt,
{
char *type_str;
int type;
+ char *authId, *authDomain;
+ int ret;
type_str = virXPathString("string(./usage/@type)", ctxt);
if (type_str == NULL) {
@@ -94,6 +101,27 @@ virSecretDefParseUsage(xmlXPathContextPtr ctxt,
}
break;
+ case VIR_SECRET_USAGE_TYPE_CEPH:
+ authId = virXPathString("string(./usage/auth/@id)", ctxt);
+ if (!authId) {
+ virSecretReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("ceph usage specified, but auth id is
missing"));
+ return -1;
+ }
+ authDomain = virXPathString("string(./usage/auth/@domain)", ctxt);
+ if (!authDomain) {
+ VIR_FREE(authId);
+ virSecretReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("ceph usage specified, but auth domain is
missing"));
+ return -1;
+ }
+ ret = virAlloc(&def->usage.authIdDomain, strlen(authId) +
+ strlen(authDomain) + 2);
+ sprintf(def->usage.authIdDomain, "%s/%s", authId, authDomain);
+ VIR_FREE(authId);
+ VIR_FREE(authDomain);
+ break;
...which simplifies this to just
case VIR_SECRET_USAGE_TYPE_CEPH:
def->usage.volume = virXPathString("string(./usage/domain)", ctxt);
if (!def->usage.volume) {
virSecretReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Ceph usage specified, but volume domain is
missing"));
return -1;
}
break;
+
default:
virSecretReportError(VIR_ERR_INTERNAL_ERROR,
_("unexpected secret usage type %d"),
@@ -220,6 +248,9 @@ virSecretDefFormatUsage(virBufferPtr buf,
const virSecretDefPtr def)
{
const char *type;
+ char *p;
+ char idAuth[80];
+ int len;
type = virSecretUsageTypeTypeToString(def->usage_type);
if (type == NULL) {
@@ -239,6 +270,18 @@ virSecretDefFormatUsage(virBufferPtr buf,
def->usage.volume);
break;
+ case VIR_SECRET_USAGE_TYPE_CEPH:
+ if (def->usage.authIdDomain != NULL) {
+ p = strchr(def->usage.authIdDomain, '/');
+ len = p - def->usage.authIdDomain;
+ strncpy(idAuth, def->usage.authIdDomain, len);
+ idAuth[len] = '\0';
+ p++;
+ virBufferEscapeString(buf, " <auth id='%s'",
idAuth);
+ virBufferEscapeString(buf, " domain='%s'/>\n", p);
+ }
+ break;
Likewise this to just
virBufferEscapeString(buf, " <domain>%s</domain>\n",
def->usage.authIdDomain);
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
11:13 p.m.
New subject: [libvirt] [PATCH 2/5] storage: add authId, authDomain to virDomainDiskDef
Add additional fields to let you specify the how to authenticate with a
network disk type. The authId is the name to authenticate as, and the
authDomain optionally describes the domain that user exists in. The latter
allows us to locate a secret in using the libvirt secrets API, as the user
is may not unique if libvirt is talking to multiple backend clusters.
Signed-off-by: Sage Weil <sage(a)newdream.net>
---
docs/schemas/domain.rng | 6 ++++++
src/conf/domain_conf.c | 43 ++++++++++++++++++++++++++++++++++---------
src/conf/domain_conf.h | 2 ++
3 files changed, 42 insertions(+), 9 deletions(-)
diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng
index 6ccbeed..3574f03 100644
--- a/docs/schemas/domain.rng
+++ b/docs/schemas/domain.rng
@@ -736,6 +736,12 @@
</attribute>
<optional>
<attribute name="name"/>
+ <element name="auth">
+ <attribute name="id"/>
+ <optional>
+ <attribute name="domain"/>
+ </optional>
+ </element>
</optional>
<zeroOrMore>
<element name="host">
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 010ce57..5b80a9e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2066,7 +2066,7 @@ virDomainDiskDefParseXML(virCapsPtr caps,
unsigned int flags)
{
virDomainDiskDefPtr def;
- xmlNodePtr cur, host;
+ xmlNodePtr cur, child;
char *type = NULL;
char *device = NULL;
char *driverName = NULL;
@@ -2084,6 +2084,8 @@ virDomainDiskDefParseXML(virCapsPtr caps,
char *devaddr = NULL;
virStorageEncryptionPtr encryption = NULL;
char *serial = NULL;
+ char *authId = NULL;
+ char *authDomain = NULL;
if (VIR_ALLOC(def) < 0) {
virReportOOMError();
@@ -2137,10 +2139,10 @@ virDomainDiskDefParseXML(virCapsPtr caps,
_("missing name for disk
source"));
goto error;
}
- host = cur->children;
- while (host != NULL) {
- if (host->type == XML_ELEMENT_NODE &&
- xmlStrEqual(host->name, BAD_CAST "host")) {
+ child = cur->children;
+ while (child != NULL) {
+ if (child->type == XML_ELEMENT_NODE &&
+ xmlStrEqual(child->name, BAD_CAST "host")) {
if (VIR_REALLOC_N(hosts, nhosts + 1) < 0) {
virReportOOMError();
goto error;
@@ -2149,20 +2151,30 @@ virDomainDiskDefParseXML(virCapsPtr caps,
hosts[nhosts].port = NULL;
nhosts++;
- hosts[nhosts - 1].name = virXMLPropString(host,
"name");
+ hosts[nhosts - 1].name = virXMLPropString(child,
"name");
if (!hosts[nhosts - 1].name) {
virDomainReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("missing name
for host"));
goto error;
}
- hosts[nhosts - 1].port = virXMLPropString(host,
"port");
+ hosts[nhosts - 1].port = virXMLPropString(child,
"port");
if (!hosts[nhosts - 1].port) {
virDomainReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("missing port
for host"));
goto error;
}
}
- host = host->next;
+ if (child->type == XML_ELEMENT_NODE &&
+ xmlStrEqual(child->name, BAD_CAST "auth")) {
+ authId = virXMLPropString(child, "id");
+ if (!authId) {
+ virDomainReportError(VIR_ERR_INTERNAL_ERROR,
+ "%s", _("missing id
for auth"));
+ goto error;
+ }
+ authDomain = virXMLPropString(child, "domain");
+ }
+ child = child->next;
}
break;
default:
@@ -2373,6 +2385,10 @@ virDomainDiskDefParseXML(virCapsPtr caps,
hosts = NULL;
def->nhosts = nhosts;
nhosts = 0;
+ def->authId = authId;
+ authId = NULL;
+ def->authDomain = authDomain;
+ authDomain = NULL;
def->driverName = driverName;
driverName = NULL;
def->driverType = driverType;
@@ -2408,6 +2424,8 @@ cleanup:
VIR_FREE(hosts);
VIR_FREE(protocol);
VIR_FREE(device);
+ VIR_FREE(authDomain);
+ VIR_FREE(authId);
VIR_FREE(driverType);
VIR_FREE(driverName);
VIR_FREE(cachetag);
@@ -8645,12 +8663,19 @@ virDomainDiskDefFormat(virBufferPtr buf,
if (def->src) {
virBufferEscapeString(buf, " name='%s'", def->src);
}
- if (def->nhosts == 0) {
+ if (def->nhosts == 0 && def->authId == NULL) {
virBufferAsprintf(buf, "/>\n");
} else {
int i;
virBufferAsprintf(buf, ">\n");
+ if (def->authId) {
+ virBufferAsprintf(buf, " <auth id='%s'",
+ def->authId);
+ if (def->authDomain)
+ virBufferAsprintf(buf, " domain='%s'",
def->authDomain);
+ virBufferStrcat(buf, "/>\n", NULL);
+ }
for (i = 0; i < def->nhosts; i++) {
virBufferEscapeString(buf, " <host
name='%s'",
def->hosts[i].name);
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index abf9cbd..8a997e1 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -221,6 +221,8 @@ struct _virDomainDiskDef {
int protocol;
int nhosts;
virDomainDiskHostDefPtr hosts;
+ char *authDomain; /* ceph cluster name */
+ char *authId; /* ceph auth id */
char *driverName;
char *driverType;
char *serial;
--
1.7.4.1
11:38 a.m.
New subject: [libvirt] [PATCH 2/5] storage: add authId, authDomain to virDomainDiskDef
On Mon, Sep 19, 2011 at 09:13:40PM -0700, Sage Weil wrote:
Add additional fields to let you specify the how to authenticate with
a
network disk type. The authId is the name to authenticate as, and the
authDomain optionally describes the domain that user exists in. The latter
allows us to locate a secret in using the libvirt secrets API, as the user
is may not unique if libvirt is talking to multiple backend clusters.
Signed-off-by: Sage Weil <sage(a)newdream.net>
---
docs/schemas/domain.rng | 6 ++++++
src/conf/domain_conf.c | 43 ++++++++++++++++++++++++++++++++++---------
src/conf/domain_conf.h | 2 ++
3 files changed, 42 insertions(+), 9 deletions(-)
diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng
index 6ccbeed..3574f03 100644
--- a/docs/schemas/domain.rng
+++ b/docs/schemas/domain.rng
@@ -736,6 +736,12 @@
</attribute>
<optional>
<attribute name="name"/>
+ <element name="auth">
+ <attribute name="id"/>
+ <optional>
+ <attribute name="domain"/>
+ </optional>
+ </element>
</optional>
<zeroOrMore>
<element name="host">
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 010ce57..5b80a9e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2066,7 +2066,7 @@ virDomainDiskDefParseXML(virCapsPtr caps,
unsigned int flags)
{
virDomainDiskDefPtr def;
- xmlNodePtr cur, host;
+ xmlNodePtr cur, child;
char *type = NULL;
char *device = NULL;
char *driverName = NULL;
@@ -2084,6 +2084,8 @@ virDomainDiskDefParseXML(virCapsPtr caps,
char *devaddr = NULL;
virStorageEncryptionPtr encryption = NULL;
char *serial = NULL;
+ char *authId = NULL;
+ char *authDomain = NULL;
if (VIR_ALLOC(def) < 0) {
virReportOOMError();
@@ -2137,10 +2139,10 @@ virDomainDiskDefParseXML(virCapsPtr caps,
_("missing name for disk
source"));
goto error;
}
- host = cur->children;
- while (host != NULL) {
- if (host->type == XML_ELEMENT_NODE &&
- xmlStrEqual(host->name, BAD_CAST "host")) {
+ child = cur->children;
+ while (child != NULL) {
+ if (child->type == XML_ELEMENT_NODE &&
+ xmlStrEqual(child->name, BAD_CAST "host")) {
if (VIR_REALLOC_N(hosts, nhosts + 1) < 0) {
virReportOOMError();
goto error;
@@ -2149,20 +2151,30 @@ virDomainDiskDefParseXML(virCapsPtr caps,
hosts[nhosts].port = NULL;
nhosts++;
- hosts[nhosts - 1].name = virXMLPropString(host,
"name");
+ hosts[nhosts - 1].name = virXMLPropString(child,
"name");
if (!hosts[nhosts - 1].name) {
virDomainReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("missing
name for host"));
goto error;
}
- hosts[nhosts - 1].port = virXMLPropString(host,
"port");
+ hosts[nhosts - 1].port = virXMLPropString(child,
"port");
if (!hosts[nhosts - 1].port) {
virDomainReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("missing
port for host"));
goto error;
}
}
- host = host->next;
+ if (child->type == XML_ELEMENT_NODE &&
+ xmlStrEqual(child->name, BAD_CAST "auth")) {
+ authId = virXMLPropString(child, "id");
+ if (!authId) {
+ virDomainReportError(VIR_ERR_INTERNAL_ERROR,
+ "%s", _("missing id
for auth"));
+ goto error;
+ }
+ authDomain = virXMLPropString(child, "domain");
+ }
+ child = child->next;
}
break;
default:
@@ -2373,6 +2385,10 @@ virDomainDiskDefParseXML(virCapsPtr caps,
hosts = NULL;
def->nhosts = nhosts;
nhosts = 0;
+ def->authId = authId;
+ authId = NULL;
+ def->authDomain = authDomain;
+ authDomain = NULL;
def->driverName = driverName;
driverName = NULL;
def->driverType = driverType;
@@ -2408,6 +2424,8 @@ cleanup:
VIR_FREE(hosts);
VIR_FREE(protocol);
VIR_FREE(device);
+ VIR_FREE(authDomain);
+ VIR_FREE(authId);
VIR_FREE(driverType);
VIR_FREE(driverName);
VIR_FREE(cachetag);
@@ -8645,12 +8663,19 @@ virDomainDiskDefFormat(virBufferPtr buf,
if (def->src) {
virBufferEscapeString(buf, " name='%s'",
def->src);
}
- if (def->nhosts == 0) {
+ if (def->nhosts == 0 && def->authId == NULL) {
virBufferAsprintf(buf, "/>\n");
} else {
int i;
virBufferAsprintf(buf, ">\n");
+ if (def->authId) {
+ virBufferAsprintf(buf, " <auth
id='%s'",
+ def->authId);
+ if (def->authDomain)
+ virBufferAsprintf(buf, " domain='%s'",
def->authDomain);
+ virBufferStrcat(buf, "/>\n", NULL);
+ }
for (i = 0; i < def->nhosts; i++) {
virBufferEscapeString(buf, " <host
name='%s'",
def->hosts[i].name);
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index abf9cbd..8a997e1 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -221,6 +221,8 @@ struct _virDomainDiskDef {
int protocol;
int nhosts;
virDomainDiskHostDefPtr hosts;
+ char *authDomain; /* ceph cluster name */
+ char *authId; /* ceph auth id */
char *driverName;
char *driverType;
char *serial;
Based on my comments in patch 0, I would suggest something along the
lines of:
enum virDomainDiskSecretType {
VIR_DOMAIN_DISK_SECRET_TYPE_NONE,
VIR_DOMAIN_DISK_SECRET_TYPE_UUID,
VIR_DOMAIN_DISK_SECRET_TYPE_USAGE,
};
struct {
char *username;
int secretType;
union {
unsigned char uuid[VIR_UUID_BUFLEN];
char *usage;
} secret;
} auth;
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
11:13 p.m.
New subject: [libvirt] [PATCH 3/5] qemu: pass virConnectPtr into Domain{Attach, Detach}*
The qemu RBD driver needs access to the conn in order to get the secret
needed for connecting to the ceph cluster.
Signed-off-by: Sage Weil <sage(a)newdream.net>
---
src/qemu/qemu_command.c | 5 +++--
src/qemu/qemu_command.h | 3 ++-
src/qemu/qemu_driver.c | 17 ++++++++---------
src/qemu/qemu_hotplug.c | 15 +++++++++------
src/qemu/qemu_hotplug.h | 9 ++++++---
5 files changed, 28 insertions(+), 21 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 76df0aa..32dcb79 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1336,7 +1336,8 @@ qemuSafeSerialParamValue(const char *value)
char *
-qemuBuildDriveStr(virDomainDiskDefPtr disk,
+qemuBuildDriveStr(virConnectPtr conn,
+ virDomainDiskDefPtr disk,
bool bootable,
virBitmapPtr qemuCaps)
{
@@ -3460,7 +3461,7 @@ qemuBuildCommandLine(virConnectPtr conn,
deviceFlagMasked = true;
}
}
- optstr = qemuBuildDriveStr(disk,
+ optstr = qemuBuildDriveStr(conn, disk,
emitBootindex ? false : !!bootindex,
qemuCaps);
if (deviceFlagMasked)
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index 87660f2..4a90544 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -75,7 +75,8 @@ char *qemuDeviceDriveHostAlias(virDomainDiskDefPtr disk,
virBitmapPtr qemuCaps);
/* Both legacy & current support */
-char *qemuBuildDriveStr(virDomainDiskDefPtr disk,
+char *qemuBuildDriveStr(virConnectPtr conn,
+ virDomainDiskDefPtr disk,
bool bootable,
virBitmapPtr qemuCaps);
char *qemuBuildFSStr(virDomainFSDefPtr fs,
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ce19be7..a1b73e7 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4773,7 +4773,8 @@ qemudDomainUndefine(virDomainPtr dom)
}
static int
-qemuDomainAttachDeviceDiskLive(struct qemud_driver *driver,
+qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
+ struct qemud_driver *driver,
virDomainObjPtr vm,
virDomainDeviceDefPtr dev)
{
@@ -4805,12 +4806,12 @@ qemuDomainAttachDeviceDiskLive(struct qemud_driver *driver,
break;
case VIR_DOMAIN_DISK_DEVICE_DISK:
if (disk->bus == VIR_DOMAIN_DISK_BUS_USB)
- ret = qemuDomainAttachUsbMassstorageDevice(driver, vm,
+ ret = qemuDomainAttachUsbMassstorageDevice(conn, driver, vm,
disk);
else if (disk->bus == VIR_DOMAIN_DISK_BUS_VIRTIO)
- ret = qemuDomainAttachPciDiskDevice(driver, vm, disk);
+ ret = qemuDomainAttachPciDiskDevice(conn, driver, vm, disk);
else if (disk->bus == VIR_DOMAIN_DISK_BUS_SCSI)
- ret = qemuDomainAttachSCSIDisk(driver, vm, disk);
+ ret = qemuDomainAttachSCSIDisk(conn, driver, vm, disk);
else
qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("disk bus '%s' cannot be hotplugged."),
@@ -4866,7 +4867,7 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
switch (dev->type) {
case VIR_DOMAIN_DEVICE_DISK:
qemuDomainObjCheckDiskTaint(driver, vm, dev->data.disk, -1);
- ret = qemuDomainAttachDeviceDiskLive(driver, vm, dev);
+ ret = qemuDomainAttachDeviceDiskLive(dom->conn, driver, vm, dev);
if (!ret)
dev->data.disk = NULL;
break;
@@ -5396,8 +5397,7 @@ static int qemuDomainAttachDeviceFlags(virDomainPtr dom, const char
*xml,
static int qemuDomainAttachDevice(virDomainPtr dom, const char *xml)
{
- return qemuDomainAttachDeviceFlags(dom, xml,
- VIR_DOMAIN_AFFECT_LIVE);
+ return qemuDomainAttachDeviceFlags(dom, xml, VIR_DOMAIN_AFFECT_LIVE);
}
@@ -5416,8 +5416,7 @@ static int qemuDomainDetachDeviceFlags(virDomainPtr dom, const char
*xml,
static int qemuDomainDetachDevice(virDomainPtr dom, const char *xml)
{
- return qemuDomainDetachDeviceFlags(dom, xml,
- VIR_DOMAIN_AFFECT_LIVE);
+ return qemuDomainDetachDeviceFlags(dom, xml, VIR_DOMAIN_AFFECT_LIVE);
}
static int qemudDomainGetAutostart(virDomainPtr dom,
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 5f449fb..3568609 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -151,7 +151,8 @@ error:
}
-int qemuDomainAttachPciDiskDevice(struct qemud_driver *driver,
+int qemuDomainAttachPciDiskDevice(virConnectPtr conn,
+ struct qemud_driver *driver,
virDomainObjPtr vm,
virDomainDiskDefPtr disk)
{
@@ -187,7 +188,7 @@ int qemuDomainAttachPciDiskDevice(struct qemud_driver *driver,
if (qemuAssignDeviceDiskAlias(disk, priv->qemuCaps) < 0)
goto error;
- if (!(drivestr = qemuBuildDriveStr(disk, false, priv->qemuCaps)))
+ if (!(drivestr = qemuBuildDriveStr(conn, disk, false, priv->qemuCaps)))
goto error;
if (!(devstr = qemuBuildDriveDevStr(disk, 0, priv->qemuCaps)))
@@ -372,7 +373,8 @@ qemuDomainFindOrCreateSCSIDiskController(struct qemud_driver *driver,
}
-int qemuDomainAttachSCSIDisk(struct qemud_driver *driver,
+int qemuDomainAttachSCSIDisk(virConnectPtr conn,
+ struct qemud_driver *driver,
virDomainObjPtr vm,
virDomainDiskDefPtr disk)
{
@@ -416,7 +418,7 @@ int qemuDomainAttachSCSIDisk(struct qemud_driver *driver,
goto error;
}
- if (!(drivestr = qemuBuildDriveStr(disk, false, priv->qemuCaps)))
+ if (!(drivestr = qemuBuildDriveStr(conn, disk, false, priv->qemuCaps)))
goto error;
for (i = 0 ; i <= disk->info.addr.drive.controller ; i++) {
@@ -495,7 +497,8 @@ error:
}
-int qemuDomainAttachUsbMassstorageDevice(struct qemud_driver *driver,
+int qemuDomainAttachUsbMassstorageDevice(virConnectPtr conn,
+ struct qemud_driver *driver,
virDomainObjPtr vm,
virDomainDiskDefPtr disk)
{
@@ -532,7 +535,7 @@ int qemuDomainAttachUsbMassstorageDevice(struct qemud_driver *driver,
if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
if (qemuAssignDeviceDiskAlias(disk, priv->qemuCaps) < 0)
goto error;
- if (!(drivestr = qemuBuildDriveStr(disk, false, priv->qemuCaps)))
+ if (!(drivestr = qemuBuildDriveStr(conn, disk, false, priv->qemuCaps)))
goto error;
if (!(devstr = qemuBuildDriveDevStr(disk, 0, priv->qemuCaps)))
goto error;
diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h
index 009f1f6..14017bf 100644
--- a/src/qemu/qemu_hotplug.h
+++ b/src/qemu/qemu_hotplug.h
@@ -31,16 +31,19 @@ int qemuDomainChangeEjectableMedia(struct qemud_driver *driver,
virDomainObjPtr vm,
virDomainDiskDefPtr disk,
bool force);
-int qemuDomainAttachPciDiskDevice(struct qemud_driver *driver,
+int qemuDomainAttachPciDiskDevice(virConnectPtr conn,
+ struct qemud_driver *driver,
virDomainObjPtr vm,
virDomainDiskDefPtr disk);
int qemuDomainAttachPciControllerDevice(struct qemud_driver *driver,
virDomainObjPtr vm,
virDomainControllerDefPtr controller);
-int qemuDomainAttachSCSIDisk(struct qemud_driver *driver,
+int qemuDomainAttachSCSIDisk(virConnectPtr conn,
+ struct qemud_driver *driver,
virDomainObjPtr vm,
virDomainDiskDefPtr disk);
-int qemuDomainAttachUsbMassstorageDevice(struct qemud_driver *driver,
+int qemuDomainAttachUsbMassstorageDevice(virConnectPtr conn,
+ struct qemud_driver *driver,
virDomainObjPtr vm,
virDomainDiskDefPtr disk);
int qemuDomainAttachNetDevice(virConnectPtr conn,
--
1.7.4.1
11:40 a.m.
New subject: [libvirt] [PATCH 3/5] qemu: pass virConnectPtr into Domain{Attach, Detach}*
On Mon, Sep 19, 2011 at 09:13:41PM -0700, Sage Weil wrote:
The qemu RBD driver needs access to the conn in order to get the
secret
needed for connecting to the ceph cluster.
Signed-off-by: Sage Weil <sage(a)newdream.net>
---
src/qemu/qemu_command.c | 5 +++--
src/qemu/qemu_command.h | 3 ++-
src/qemu/qemu_driver.c | 17 ++++++++---------
src/qemu/qemu_hotplug.c | 15 +++++++++------
src/qemu/qemu_hotplug.h | 9 ++++++---
5 files changed, 28 insertions(+), 21 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 76df0aa..32dcb79 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1336,7 +1336,8 @@ qemuSafeSerialParamValue(const char *value)
char *
-qemuBuildDriveStr(virDomainDiskDefPtr disk,
+qemuBuildDriveStr(virConnectPtr conn,
+ virDomainDiskDefPtr disk,
bool bootable,
virBitmapPtr qemuCaps)
{
@@ -3460,7 +3461,7 @@ qemuBuildCommandLine(virConnectPtr conn,
deviceFlagMasked = true;
}
}
- optstr = qemuBuildDriveStr(disk,
+ optstr = qemuBuildDriveStr(conn, disk,
emitBootindex ? false : !!bootindex,
qemuCaps);
if (deviceFlagMasked)
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index 87660f2..4a90544 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -75,7 +75,8 @@ char *qemuDeviceDriveHostAlias(virDomainDiskDefPtr disk,
virBitmapPtr qemuCaps);
/* Both legacy & current support */
-char *qemuBuildDriveStr(virDomainDiskDefPtr disk,
+char *qemuBuildDriveStr(virConnectPtr conn,
+ virDomainDiskDefPtr disk,
bool bootable,
virBitmapPtr qemuCaps);
char *qemuBuildFSStr(virDomainFSDefPtr fs,
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ce19be7..a1b73e7 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4773,7 +4773,8 @@ qemudDomainUndefine(virDomainPtr dom)
}
static int
-qemuDomainAttachDeviceDiskLive(struct qemud_driver *driver,
+qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
+ struct qemud_driver *driver,
virDomainObjPtr vm,
virDomainDeviceDefPtr dev)
{
@@ -4805,12 +4806,12 @@ qemuDomainAttachDeviceDiskLive(struct qemud_driver *driver,
break;
case VIR_DOMAIN_DISK_DEVICE_DISK:
if (disk->bus == VIR_DOMAIN_DISK_BUS_USB)
- ret = qemuDomainAttachUsbMassstorageDevice(driver, vm,
+ ret = qemuDomainAttachUsbMassstorageDevice(conn, driver, vm,
disk);
else if (disk->bus == VIR_DOMAIN_DISK_BUS_VIRTIO)
- ret = qemuDomainAttachPciDiskDevice(driver, vm, disk);
+ ret = qemuDomainAttachPciDiskDevice(conn, driver, vm, disk);
else if (disk->bus == VIR_DOMAIN_DISK_BUS_SCSI)
- ret = qemuDomainAttachSCSIDisk(driver, vm, disk);
+ ret = qemuDomainAttachSCSIDisk(conn, driver, vm, disk);
else
qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("disk bus '%s' cannot be hotplugged."),
@@ -4866,7 +4867,7 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
switch (dev->type) {
case VIR_DOMAIN_DEVICE_DISK:
qemuDomainObjCheckDiskTaint(driver, vm, dev->data.disk, -1);
- ret = qemuDomainAttachDeviceDiskLive(driver, vm, dev);
+ ret = qemuDomainAttachDeviceDiskLive(dom->conn, driver, vm, dev);
if (!ret)
dev->data.disk = NULL;
break;
@@ -5396,8 +5397,7 @@ static int qemuDomainAttachDeviceFlags(virDomainPtr dom, const char
*xml,
static int qemuDomainAttachDevice(virDomainPtr dom, const char *xml)
{
- return qemuDomainAttachDeviceFlags(dom, xml,
- VIR_DOMAIN_AFFECT_LIVE);
+ return qemuDomainAttachDeviceFlags(dom, xml, VIR_DOMAIN_AFFECT_LIVE);
}
@@ -5416,8 +5416,7 @@ static int qemuDomainDetachDeviceFlags(virDomainPtr dom, const char
*xml,
static int qemuDomainDetachDevice(virDomainPtr dom, const char *xml)
{
- return qemuDomainDetachDeviceFlags(dom, xml,
- VIR_DOMAIN_AFFECT_LIVE);
+ return qemuDomainDetachDeviceFlags(dom, xml, VIR_DOMAIN_AFFECT_LIVE);
}
static int qemudDomainGetAutostart(virDomainPtr dom,
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 5f449fb..3568609 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -151,7 +151,8 @@ error:
}
-int qemuDomainAttachPciDiskDevice(struct qemud_driver *driver,
+int qemuDomainAttachPciDiskDevice(virConnectPtr conn,
+ struct qemud_driver *driver,
virDomainObjPtr vm,
virDomainDiskDefPtr disk)
{
@@ -187,7 +188,7 @@ int qemuDomainAttachPciDiskDevice(struct qemud_driver *driver,
if (qemuAssignDeviceDiskAlias(disk, priv->qemuCaps) < 0)
goto error;
- if (!(drivestr = qemuBuildDriveStr(disk, false, priv->qemuCaps)))
+ if (!(drivestr = qemuBuildDriveStr(conn, disk, false, priv->qemuCaps)))
goto error;
if (!(devstr = qemuBuildDriveDevStr(disk, 0, priv->qemuCaps)))
@@ -372,7 +373,8 @@ qemuDomainFindOrCreateSCSIDiskController(struct qemud_driver
*driver,
}
-int qemuDomainAttachSCSIDisk(struct qemud_driver *driver,
+int qemuDomainAttachSCSIDisk(virConnectPtr conn,
+ struct qemud_driver *driver,
virDomainObjPtr vm,
virDomainDiskDefPtr disk)
{
@@ -416,7 +418,7 @@ int qemuDomainAttachSCSIDisk(struct qemud_driver *driver,
goto error;
}
- if (!(drivestr = qemuBuildDriveStr(disk, false, priv->qemuCaps)))
+ if (!(drivestr = qemuBuildDriveStr(conn, disk, false, priv->qemuCaps)))
goto error;
for (i = 0 ; i <= disk->info.addr.drive.controller ; i++) {
@@ -495,7 +497,8 @@ error:
}
-int qemuDomainAttachUsbMassstorageDevice(struct qemud_driver *driver,
+int qemuDomainAttachUsbMassstorageDevice(virConnectPtr conn,
+ struct qemud_driver *driver,
virDomainObjPtr vm,
virDomainDiskDefPtr disk)
{
@@ -532,7 +535,7 @@ int qemuDomainAttachUsbMassstorageDevice(struct qemud_driver
*driver,
if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
if (qemuAssignDeviceDiskAlias(disk, priv->qemuCaps) < 0)
goto error;
- if (!(drivestr = qemuBuildDriveStr(disk, false, priv->qemuCaps)))
+ if (!(drivestr = qemuBuildDriveStr(conn, disk, false, priv->qemuCaps)))
goto error;
if (!(devstr = qemuBuildDriveDevStr(disk, 0, priv->qemuCaps)))
goto error;
diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h
index 009f1f6..14017bf 100644
--- a/src/qemu/qemu_hotplug.h
+++ b/src/qemu/qemu_hotplug.h
@@ -31,16 +31,19 @@ int qemuDomainChangeEjectableMedia(struct qemud_driver *driver,
virDomainObjPtr vm,
virDomainDiskDefPtr disk,
bool force);
-int qemuDomainAttachPciDiskDevice(struct qemud_driver *driver,
+int qemuDomainAttachPciDiskDevice(virConnectPtr conn,
+ struct qemud_driver *driver,
virDomainObjPtr vm,
virDomainDiskDefPtr disk);
int qemuDomainAttachPciControllerDevice(struct qemud_driver *driver,
virDomainObjPtr vm,
virDomainControllerDefPtr controller);
-int qemuDomainAttachSCSIDisk(struct qemud_driver *driver,
+int qemuDomainAttachSCSIDisk(virConnectPtr conn,
+ struct qemud_driver *driver,
virDomainObjPtr vm,
virDomainDiskDefPtr disk);
-int qemuDomainAttachUsbMassstorageDevice(struct qemud_driver *driver,
+int qemuDomainAttachUsbMassstorageDevice(virConnectPtr conn,
+ struct qemud_driver *driver,
virDomainObjPtr vm,
virDomainDiskDefPtr disk);
int qemuDomainAttachNetDevice(virConnectPtr conn,
ACK, this all looks fine. In fact given that it wasn't there already,
I have a feeling that hotplug of qcow2 encrypted disks is currently
broken, which is something I need to look into.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
11:13 p.m.
New subject: [libvirt] [PATCH 4/5] buf: implement generic virBufferEscape
Implement a generic helper to escape a given set of characters with a
leading '\'. Generalizes virBufferEscapeSexpr().
Signed-off-by: Sage Weil <sage(a)newdream.net>
---
src/libvirt_private.syms | 1 +
src/util/buf.c | 33 ++++++++++++++++++++++++++-------
src/util/buf.h | 1 +
3 files changed, 28 insertions(+), 7 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 830222b..d230fab 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -25,6 +25,7 @@ virBufferAddChar;
virBufferAsprintf;
virBufferContentAndReset;
virBufferError;
+virBufferEscape;
virBufferEscapeSexpr;
virBufferEscapeString;
virBufferFreeAndReset;
diff --git a/src/util/buf.c b/src/util/buf.c
index 5002486..7d0d2d3 100644
--- a/src/util/buf.c
+++ b/src/util/buf.c
@@ -383,9 +383,29 @@ virBufferEscapeSexpr(const virBufferPtr buf,
const char *format,
const char *str)
{
+ virBufferEscape(buf, "\\\'", format, str);
+}
+
+/**
+ * virBufferEscape:
+ * @buf: the buffer to dump
+ * @toescape: NULL-terminated list of characters to escape
+ * @format: a printf like format string but with only one %s parameter
+ * @str: the string argument which need to be escaped
+ *
+ * Do a formatted print with a single string to a buffer. Any characters
+ * in the provided list are escaped with a preceeding \.
+ */
+void
+virBufferEscape(const virBufferPtr buf,
+ const char *toescape,
+ const char *format,
+ const char *str)
+{
int len;
char *escaped, *out;
const char *cur;
+ const char *p;
if ((format == NULL) || (buf == NULL) || (str == NULL))
return;
@@ -408,14 +428,13 @@ virBufferEscapeSexpr(const virBufferPtr buf,
cur = str;
out = escaped;
while (*cur != 0) {
- switch (*cur) {
- case '\\':
- case '\'':
- *out++ = '\\';
- /* fallthrough */
- default:
- *out++ = *cur;
+ for (p = toescape; *p; ++p) {
+ if (*cur == *p) {
+ *out++ = '\\';
+ break;
+ }
}
+ *out++ = *cur;
cur++;
}
*out = 0;
diff --git a/src/util/buf.h b/src/util/buf.h
index 06d01ba..e545ed9 100644
--- a/src/util/buf.h
+++ b/src/util/buf.h
@@ -50,6 +50,7 @@ void virBufferStrcat(const virBufferPtr buf, ...)
ATTRIBUTE_SENTINEL;
void virBufferEscapeString(const virBufferPtr buf, const char *format, const char *str);
void virBufferEscapeSexpr(const virBufferPtr buf, const char *format, const char *str);
+void virBufferEscape(const virBufferPtr buf, const char *toescape, const char *format,
const char *str);
void virBufferURIEncodeString (const virBufferPtr buf, const char *str);
# define virBufferAddLit(buf_, literal_string_) \
--
1.7.4.1
5:39 a.m.
New subject: [libvirt] [PATCH 4/5] buf: implement generic virBufferEscape
On Mon, Sep 19, 2011 at 09:13:42PM -0700, Sage Weil wrote:
Implement a generic helper to escape a given set of characters with
a
leading '\'. Generalizes virBufferEscapeSexpr().
Signed-off-by: Sage Weil <sage(a)newdream.net>
---
src/libvirt_private.syms | 1 +
src/util/buf.c | 33 ++++++++++++++++++++++++++-------
src/util/buf.h | 1 +
3 files changed, 28 insertions(+), 7 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 830222b..d230fab 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -25,6 +25,7 @@ virBufferAddChar;
virBufferAsprintf;
virBufferContentAndReset;
virBufferError;
+virBufferEscape;
virBufferEscapeSexpr;
virBufferEscapeString;
virBufferFreeAndReset;
diff --git a/src/util/buf.c b/src/util/buf.c
index 5002486..7d0d2d3 100644
--- a/src/util/buf.c
+++ b/src/util/buf.c
@@ -383,9 +383,29 @@ virBufferEscapeSexpr(const virBufferPtr buf,
const char *format,
const char *str)
{
+ virBufferEscape(buf, "\\\'", format, str);
+}
+
+/**
+ * virBufferEscape:
+ * @buf: the buffer to dump
+ * @toescape: NULL-terminated list of characters to escape
+ * @format: a printf like format string but with only one %s parameter
+ * @str: the string argument which need to be escaped
+ *
+ * Do a formatted print with a single string to a buffer. Any characters
+ * in the provided list are escaped with a preceeding \.
+ */
+void
+virBufferEscape(const virBufferPtr buf,
+ const char *toescape,
+ const char *format,
+ const char *str)
+{
int len;
char *escaped, *out;
const char *cur;
+ const char *p;
if ((format == NULL) || (buf == NULL) || (str == NULL))
return;
@@ -408,14 +428,13 @@ virBufferEscapeSexpr(const virBufferPtr buf,
cur = str;
out = escaped;
while (*cur != 0) {
- switch (*cur) {
- case '\\':
- case '\'':
- *out++ = '\\';
- /* fallthrough */
- default:
- *out++ = *cur;
+ for (p = toescape; *p; ++p) {
+ if (*cur == *p) {
+ *out++ = '\\';
+ break;
+ }
}
+ *out++ = *cur;
cur++;
}
*out = 0;
diff --git a/src/util/buf.h b/src/util/buf.h
index 06d01ba..e545ed9 100644
--- a/src/util/buf.h
+++ b/src/util/buf.h
@@ -50,6 +50,7 @@ void virBufferStrcat(const virBufferPtr buf, ...)
ATTRIBUTE_SENTINEL;
void virBufferEscapeString(const virBufferPtr buf, const char *format, const char
*str);
void virBufferEscapeSexpr(const virBufferPtr buf, const char *format, const char *str);
+void virBufferEscape(const virBufferPtr buf, const char *toescape, const char *format,
const char *str);
void virBufferURIEncodeString (const virBufferPtr buf, const char *str);
# define virBufferAddLit(buf_, literal_string_) \
ACK, trivial isolated patch
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
12:09 p.m.
New subject: [libvirt] [PATCH 4/5] buf: implement generic virBufferEscape
On 10/11/2011 04:39 AM, Daniel P. Berrange wrote:
On Mon, Sep 19, 2011 at 09:13:42PM -0700, Sage Weil wrote:
> Implement a generic helper to escape a given set of characters with a
> leading '\'. Generalizes virBufferEscapeSexpr().
>
> Signed-off-by: Sage Weil<sage(a)newdream.net>
> @@ -408,14 +428,13 @@ virBufferEscapeSexpr(const virBufferPtr buf,
> cur = str;
> out = escaped;
> while (*cur != 0) {
> - switch (*cur) {
> - case '\\':
> - case '\'':
> - *out++ = '\\';
> - /* fallthrough */
> - default:
> - *out++ = *cur;
> + for (p = toescape; *p; ++p) {
> + if (*cur == *p) {
strchr is slightly more efficient than hand-rolling this loop.
More importantly, you had a logic bug, where you replaced one hard-coded
instance of "\\'", but not the other (the strcspn optimization). Also,
I tend to use "'" instead of "\'", although both work, since I
favor
concise code (I save my ramblings for my emails :).
ACK, trivial isolated patch
I added you to AUTHORS, squashed this in, then pushed.
diff --git i/src/util/buf.c w/src/util/buf.c
index 7d0d2d3..5627d8f 100644
--- i/src/util/buf.c
+++ w/src/util/buf.c
@@ -383,7 +383,7 @@ virBufferEscapeSexpr(const virBufferPtr buf,
const char *format,
const char *str)
{
- virBufferEscape(buf, "\\\'", format, str);
+ virBufferEscape(buf, "\\'", format, str);
}
/**
@@ -414,7 +414,7 @@ virBufferEscape(const virBufferPtr buf,
return;
len = strlen(str);
- if (strcspn(str, "\\'") == len) {
+ if (strcspn(str, toescape) == len) {
virBufferAsprintf(buf, format, str);
return;
}
@@ -428,12 +428,8 @@ virBufferEscape(const virBufferPtr buf,
cur = str;
out = escaped;
while (*cur != 0) {
- for (p = toescape; *p; ++p) {
- if (*cur == *p) {
- *out++ = '\\';
- break;
- }
- }
+ if (strchr(toescape, *cur))
+ *out++ = '\\';
*out++ = *cur;
cur++;
}
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
11:13 p.m.
New subject: [libvirt] [PATCH 5/5] qemu/rbd: improve rbd device specification
This improves the support for qemu rbd devices by adding support for a few
key features (e.g., authentication) and cleaning up the way in which
rbd configuration options are passed to qemu.
And <auth> member of the disk source xml specifies how librbd should
authenticate. The id property is the Ceph/RBD user to authenticate as,
and the domain property is a identifier (local to libvirt) for the Ceph
cluster in question. If both are specified, we look for a libvirt secret
of type CEPH with matching id and domain properties.
The old RBD support relied on setting an environment variable to
communicate information to qemu/librbd. Instead, pass those options
explicitly to qemu. Update the qemu argument parsing and unit tests
accordingly.
Signed-off-by: Sage Weil <sage(a)newdream.net>
---
src/qemu/qemu_command.c | 268 +++++++++++---------
.../qemuxml2argv-disk-drive-network-rbd.args | 6 +-
.../qemuxml2argv-disk-drive-network-rbd.xml | 1 +
3 files changed, 157 insertions(+), 118 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 32dcb79..00fb169 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -38,6 +38,7 @@
#include "domain_audit.h"
#include "domain_conf.h"
#include "network/bridge_driver.h"
+#include "base64.h"
#include <sys/utsname.h>
#include <sys/stat.h>
@@ -1334,6 +1335,146 @@ qemuSafeSerialParamValue(const char *value)
return 0;
}
+static int buildRBDString(virConnectPtr conn,
+ virDomainDiskDefPtr disk,
+ virBufferPtr opt)
+{
+ int i;
+ char idDomain[80];
+ virSecretPtr sec;
+ char *secret;
+ size_t secret_size;
+
+ virBufferAsprintf(opt, "rbd:%s", disk->src);
+ if (disk->authId) {
+ virBufferEscape(opt, ":", ":id=%s", disk->authId);
+ }
+ if (disk->authDomain) {
+ /* look up secret */
+ snprintf(idDomain, sizeof(idDomain), "%s/%s", disk->authId,
+ disk->authDomain);
+ sec = virSecretLookupByUsage(conn,
+ VIR_SECRET_USAGE_TYPE_CEPH,
+ idDomain);
+ if (sec) {
+ char *base64;
+
+ secret = (char *)conn->secretDriver->getValue(sec, &secret_size,
0,
+ VIR_SECRET_GET_VALUE_INTERNAL_CALL);
+ /* qemu/librbd wants it base64 encoded */
+ base64_encode_alloc(secret, secret_size, &base64);
+ virBufferEscape(opt, ":",
":key=%s:auth_supported=cephx\\;none",
+ base64);
+ VIR_FREE(base64);
+ VIR_FREE(secret);
+ virUnrefSecret(sec);
+ } else {
+ VIR_WARN("rbd id '%s' domain '%s' specified but secret
not found",
+ disk->authId, disk->authDomain);
+ }
+ }
+ if (disk->nhosts > 0) {
+ virBufferStrcat(opt, ":mon_host=", NULL);
+ for (i = 0; i < disk->nhosts; ++i) {
+ if (i) {
+ virBufferStrcat(opt, "\\;", NULL);
+ }
+ if (disk->hosts[i].port) {
+ virBufferAsprintf(opt, "%s\\:%s",
+ disk->hosts[i].name,
+ disk->hosts[i].port);
+ } else {
+ virBufferAsprintf(opt, "%s", disk->hosts[i].name);
+ }
+ }
+ }
+
+ return 0;
+}
+
+static int addRBDHost(virDomainDiskDefPtr disk, char *hostport)
+{
+ char *port;
+ int ret;
+
+ disk->nhosts++;
+ ret = VIR_REALLOC_N(disk->hosts, disk->nhosts);
+ if (ret < 0) {
+ virReportOOMError();
+ return -1;
+ }
+
+ port = strstr(hostport, "\\:");
+ if (port) {
+ *port = '\0';
+ port += 2;
+ disk->hosts[disk->nhosts-1].port = strdup(port);
+ } else {
+ disk->hosts[disk->nhosts-1].port = strdup("6789");
+ }
+ disk->hosts[disk->nhosts-1].name = strdup(hostport);
+ return 0;
+}
+
+/* disk->src initially has everything after the rbd: prefix */
+static int parseRBDString(virDomainDiskDefPtr disk)
+{
+ char *options = NULL;
+ char *p, *e, *next;
+
+ p = strchr(disk->src, ':');
+ if (p) {
+ options = strdup(p + 1);
+ *p = '\0';
+ }
+
+ /* options */
+ if (!options)
+ return 0; /* all done */
+
+ p = options;
+ while (*p) {
+ /* find : delimiter or end of string */
+ for (e = p; *e && *e != ':'; ++e) {
+ if (*e == '\\') {
+ e++;
+ if (*e == '\0')
+ break;
+ }
+ }
+ if (*e == '\0') {
+ next = e; /* last kv pair */
+ } else {
+ next = e + 1;
+ *e = '\0';
+ }
+
+ if (STRPREFIX(p, "id=")) {
+ disk->authId = strdup(p + strlen("id="));
+ }
+ if (STRPREFIX(p, "mon_host=")) {
+ char *h, *sep;
+
+ h = p + strlen("mon_host=");
+ while (h < e) {
+ for (sep = h; sep < e; ++sep) {
+ if (*sep == '\\' && (sep[1] == ',' ||
+ sep[1] == ';' ||
+ sep[1] == ' ')) {
+ *sep = '\0';
+ sep += 2;
+ break;
+ }
+ }
+ addRBDHost(disk, h);
+ h = sep;
+ }
+ }
+
+ p = next;
+ }
+ return 0;
+}
char *
qemuBuildDriveStr(virConnectPtr conn,
@@ -1453,8 +1594,9 @@ qemuBuildDriveStr(virConnectPtr conn,
disk->hosts->name, disk->hosts->port);
break;
case VIR_DOMAIN_DISK_PROTOCOL_RBD:
- /* TODO: set monitor hostnames */
- virBufferAsprintf(&opt, "file=rbd:%s,", disk->src);
+ virBufferStrcat(&opt, "file=", NULL);
+ buildRBDString(conn, disk, &opt);
+ virBufferStrcat(&opt, ",", NULL);
break;
case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG:
if (disk->nhosts == 0)
@@ -2851,8 +2993,6 @@ qemuBuildCommandLine(virConnectPtr conn,
int last_good_net = -1;
bool hasHwVirt = false;
virCommandPtr cmd;
- bool has_rbd_hosts = false;
- virBuffer rbd_hosts = VIR_BUFFER_INITIALIZER;
bool emitBootindex = false;
uname_normalize(&ut);
@@ -3413,7 +3553,6 @@ qemuBuildCommandLine(virConnectPtr conn,
virDomainDiskDefPtr disk = def->disks[i];
int withDeviceArg = 0;
bool deviceFlagMasked = false;
- int j;
/* Unless we have -device, then USB disks need special
handling */
@@ -3471,26 +3610,6 @@ qemuBuildCommandLine(virConnectPtr conn,
virCommandAddArg(cmd, optstr);
VIR_FREE(optstr);
- if (disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK &&
- disk->protocol == VIR_DOMAIN_DISK_PROTOCOL_RBD) {
- for (j = 0 ; j < disk->nhosts ; j++) {
- if (!has_rbd_hosts) {
- virBufferAddLit(&rbd_hosts, "CEPH_ARGS=-m ");
- has_rbd_hosts = true;
- } else {
- virBufferAddLit(&rbd_hosts, ",");
- }
- virDomainDiskHostDefPtr host = &disk->hosts[j];
- if (host->port) {
- virBufferAsprintf(&rbd_hosts, "%s:%s",
- host->name,
- host->port);
- } else {
- virBufferAdd(&rbd_hosts, host->name, -1);
- }
- }
- }
-
if (!emitBootindex)
bootindex = 0;
else if (disk->bootIndex)
@@ -3528,7 +3647,6 @@ qemuBuildCommandLine(virConnectPtr conn,
char *file;
const char *fmt;
virDomainDiskDefPtr disk = def->disks[i];
- int j;
if (disk->bus == VIR_DOMAIN_DISK_BUS_USB) {
if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) {
@@ -3597,24 +3715,11 @@ qemuBuildCommandLine(virConnectPtr conn,
}
break;
case VIR_DOMAIN_DISK_PROTOCOL_RBD:
- if (virAsprintf(&file, "rbd:%s,", disk->src) < 0)
{
- goto no_memory;
- }
- for (j = 0 ; j < disk->nhosts ; j++) {
- if (!has_rbd_hosts) {
- virBufferAddLit(&rbd_hosts, "CEPH_ARGS=-m ");
- has_rbd_hosts = true;
- } else {
- virBufferAddLit(&rbd_hosts, ",");
- }
- virDomainDiskHostDefPtr host = &disk->hosts[j];
- if (host->port) {
- virBufferAsprintf(&rbd_hosts, "%s:%s",
- host->name,
- host->port);
- } else {
- virBufferAdd(&rbd_hosts, host->name, -1);
- }
+ {
+ virBuffer opt = VIR_BUFFER_INITIALIZER;
+ buildRBDString(conn, disk, &opt);
+ virAsprintf(&file, "%s",
+ virBufferContentAndReset(&opt));
}
break;
case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG:
@@ -3643,9 +3748,6 @@ qemuBuildCommandLine(virConnectPtr conn,
}
}
- if (has_rbd_hosts)
- virCommandAddEnvBuffer(cmd, &rbd_hosts);
-
if (qemuCapsGet(qemuCaps, QEMU_CAPS_FSDEV)) {
for (i = 0 ; i < def->nfss ; i++) {
char *optstr;
@@ -4777,7 +4879,6 @@ qemuBuildCommandLine(virConnectPtr conn,
networkReleaseActualDevice(def->nets[i]);
for (i = 0; i <= last_good_net; i++)
virDomainConfNWFilterTeardown(def->nets[i]);
- virBufferFreeAndReset(&rbd_hosts);
virCommandFree(cmd);
return NULL;
}
@@ -4809,10 +4910,6 @@ static int qemuStringToArgvEnv(const char *args,
const char *next;
start = curr;
- /* accept a space in CEPH_ARGS */
- if (STRPREFIX(curr, "CEPH_ARGS=-m ")) {
- start += strlen("CEPH_ARGS=-m ");
- }
if (*start == '\'') {
if (start == curr)
curr++;
@@ -5091,6 +5188,8 @@ qemuParseCommandLineDisk(virCapsPtr caps,
virReportOOMError();
goto cleanup;
}
+ if (parseRBDString(def) < 0)
+ goto cleanup;
VIR_FREE(p);
} else if (STRPREFIX(def->src, "sheepdog:")) {
@@ -6195,7 +6294,8 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps,
disk->src = NULL;
break;
case VIR_DOMAIN_DISK_PROTOCOL_RBD:
- /* handled later since the hosts for all disks are in CEPH_ARGS */
+ if (parseRBDString(disk) < 0)
+ goto error;
break;
case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG:
/* disk->src must be [vdiname] or [host]:[port]:[vdiname] */
@@ -6534,68 +6634,6 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps,
}
#undef WANT_VALUE
- if (def->ndisks > 0) {
- const char *ceph_args = qemuFindEnv(progenv, "CEPH_ARGS");
- if (ceph_args) {
- char *hosts, *port, *saveptr = NULL, *token;
- virDomainDiskDefPtr first_rbd_disk = NULL;
- for (i = 0 ; i < def->ndisks ; i++) {
- if (def->disks[i]->type == VIR_DOMAIN_DISK_TYPE_NETWORK &&
- def->disks[i]->protocol == VIR_DOMAIN_DISK_PROTOCOL_RBD) {
- first_rbd_disk = def->disks[i];
- break;
- }
- }
-
- if (!first_rbd_disk) {
- qemuReportError(VIR_ERR_INTERNAL_ERROR,
- _("CEPH_ARGS was set without an rbd disk"));
- goto error;
- }
-
- /* CEPH_ARGS should be: -m host1[:port1][,host2[:port2]]... */
- if (!STRPREFIX(ceph_args, "-m ")) {
- qemuReportError(VIR_ERR_INTERNAL_ERROR,
- _("could not parse CEPH_ARGS '%s'"),
ceph_args);
- goto error;
- }
- hosts = strdup(strchr(ceph_args, ' ') + 1);
- if (!hosts)
- goto no_memory;
- first_rbd_disk->nhosts = 0;
- token = strtok_r(hosts, ",", &saveptr);
- while (token != NULL) {
- if (VIR_REALLOC_N(first_rbd_disk->hosts, first_rbd_disk->nhosts +
1) < 0) {
- VIR_FREE(hosts);
- goto no_memory;
- }
- port = strchr(token, ':');
- if (port) {
- *port++ = '\0';
- port = strdup(port);
- if (!port) {
- VIR_FREE(hosts);
- goto no_memory;
- }
- }
- first_rbd_disk->hosts[first_rbd_disk->nhosts].port = port;
- first_rbd_disk->hosts[first_rbd_disk->nhosts].name =
strdup(token);
- if (!first_rbd_disk->hosts[first_rbd_disk->nhosts].name) {
- VIR_FREE(hosts);
- goto no_memory;
- }
- first_rbd_disk->nhosts++;
- token = strtok_r(NULL, ",", &saveptr);
- }
- VIR_FREE(hosts);
-
- if (first_rbd_disk->nhosts == 0) {
- qemuReportError(VIR_ERR_INTERNAL_ERROR,
- _("found no rbd hosts in CEPH_ARGS
'%s'"), ceph_args);
- goto error;
- }
- }
- }
if (!nographics && def->ngraphics == 0) {
virDomainGraphicsDefPtr sdl;
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.args
b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.args
index 3ab1219..68b9e95 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.args
@@ -1,6 +1,6 @@
-LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test CEPH_ARGS=-m \
-mon1.example.org:6321,mon2.example.org:6322,mon3.example.org:6322 \
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test \
/usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -monitor \
unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -drive \
-file=/dev/HostVG/QEMUGuest1,if=ide,bus=0,unit=0 -drive file=rbd:pool/image,\
+file=/dev/HostVG/QEMUGuest1,if=ide,bus=0,unit=0 -drive \
+file=rbd:pool/image:id=myname:mon_host=mon1.example.org\:6321\;mon2.example.org\:6322\;mon3.example.org\:6322,\
if=virtio,format=raw -net none -serial none -parallel none -usb
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.xml
b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.xml
index e920db1..f090834 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.xml
@@ -22,6 +22,7 @@
<disk type='network' device='disk'>
<driver name='qemu' type='raw'/>
<source protocol='rbd' name='pool/image'>
+ <auth id='myname'/>
<host name='mon1.example.org' port='6321'/>
<host name='mon2.example.org' port='6322'/>
<host name='mon3.example.org' port='6322'/>
--
1.7.4.1
11:48 a.m.
New subject: [libvirt] [PATCH 5/5] qemu/rbd: improve rbd device specification
On Mon, Sep 19, 2011 at 09:13:43PM -0700, Sage Weil wrote:
This improves the support for qemu rbd devices by adding support for
a few
key features (e.g., authentication) and cleaning up the way in which
rbd configuration options are passed to qemu.
And <auth> member of the disk source xml specifies how librbd should
authenticate. The id property is the Ceph/RBD user to authenticate as,
and the domain property is a identifier (local to libvirt) for the Ceph
cluster in question. If both are specified, we look for a libvirt secret
of type CEPH with matching id and domain properties.
The old RBD support relied on setting an environment variable to
communicate information to qemu/librbd. Instead, pass those options
explicitly to qemu. Update the qemu argument parsing and unit tests
accordingly.
Signed-off-by: Sage Weil <sage(a)newdream.net>
---
src/qemu/qemu_command.c | 268 +++++++++++---------
.../qemuxml2argv-disk-drive-network-rbd.args | 6 +-
.../qemuxml2argv-disk-drive-network-rbd.xml | 1 +
3 files changed, 157 insertions(+), 118 deletions(-)
+static int buildRBDString(virConnectPtr conn,
+ virDomainDiskDefPtr disk,
+ virBufferPtr opt)
+{
+ int i;
+ char idDomain[80];
+ virSecretPtr sec;
+ char *secret;
+ size_t secret_size;
+
+ virBufferAsprintf(opt, "rbd:%s", disk->src);
+ if (disk->authId) {
+ virBufferEscape(opt, ":", ":id=%s", disk->authId);
+ }
+ if (disk->authDomain) {
+ /* look up secret */
+ snprintf(idDomain, sizeof(idDomain), "%s/%s", disk->authId,
+ disk->authDomain);
+ sec = virSecretLookupByUsage(conn,
+ VIR_SECRET_USAGE_TYPE_CEPH,
+ idDomain);
This is where you'd want to use virSecretLookupByUUID.
+ if (sec) {
+ char *base64;
+
+ secret = (char *)conn->secretDriver->getValue(sec, &secret_size,
0,
+ VIR_SECRET_GET_VALUE_INTERNAL_CALL);
No need for the cast to 'char *', since void * casts to anything in C.
But you do need to handle case of the return'd secret being 'NULL'
and return to the caller in that case.
+ /* qemu/librbd wants it base64 encoded */
+ base64_encode_alloc(secret, secret_size, &base64);
+ virBufferEscape(opt, ":",
":key=%s:auth_supported=cephx\\;none",
+ base64);
+ VIR_FREE(base64);
+ VIR_FREE(secret);
+ virUnrefSecret(sec);
+ } else {
+ VIR_WARN("rbd id '%s' domain '%s' specified but secret
not found",
+ disk->authId, disk->authDomain);
You ought to use qemuReportError() here and return to the caller
+ }
+ }
+ if (disk->nhosts > 0) {
+ virBufferStrcat(opt, ":mon_host=", NULL);
+ for (i = 0; i < disk->nhosts; ++i) {
+ if (i) {
+ virBufferStrcat(opt, "\\;", NULL);
+ }
+ if (disk->hosts[i].port) {
+ virBufferAsprintf(opt, "%s\\:%s",
+ disk->hosts[i].name,
+ disk->hosts[i].port);
+ } else {
+ virBufferAsprintf(opt, "%s", disk->hosts[i].name);
+ }
+ }
+ }
+
+ return 0;
+}
+
@@ -1453,8 +1594,9 @@ qemuBuildDriveStr(virConnectPtr conn,
disk->hosts->name, disk->hosts->port);
break;
case VIR_DOMAIN_DISK_PROTOCOL_RBD:
- /* TODO: set monitor hostnames */
- virBufferAsprintf(&opt, "file=rbd:%s,", disk->src);
+ virBufferStrcat(&opt, "file=", NULL);
+ buildRBDString(conn, disk, &opt);
+ virBufferStrcat(&opt, ",", NULL);
break;
case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG:
if (disk->nhosts == 0)
@@ -3597,24 +3715,11 @@ qemuBuildCommandLine(virConnectPtr conn,
}
break;
case VIR_DOMAIN_DISK_PROTOCOL_RBD:
- if (virAsprintf(&file, "rbd:%s,", disk->src) <
0) {
- goto no_memory;
- }
- for (j = 0 ; j < disk->nhosts ; j++) {
- if (!has_rbd_hosts) {
- virBufferAddLit(&rbd_hosts, "CEPH_ARGS=-m ");
- has_rbd_hosts = true;
- } else {
- virBufferAddLit(&rbd_hosts, ",");
- }
- virDomainDiskHostDefPtr host = &disk->hosts[j];
- if (host->port) {
- virBufferAsprintf(&rbd_hosts, "%s:%s",
- host->name,
- host->port);
- } else {
- virBufferAdd(&rbd_hosts, host->name, -1);
- }
+ {
+ virBuffer opt = VIR_BUFFER_INITIALIZER;
+ buildRBDString(conn, disk, &opt);
+ virAsprintf(&file, "%s",
+ virBufferContentAndReset(&opt));
This last statement leaks. 'virBufferContentAndReset' gives you back
the internal buffer to keep, so there's no need to duplicate it using
asprintf. Also you need to check for an error. So you want this
if (virBufferError(&opt)) {
virReportOOMError();
goto cleanup;
}
file = virBufferContentAndReset(&opt);
Generally I think you're going in the right direction with this
patch.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
3:40 p.m.
Hi everyone,
Any feedback on this series? I touch a few pieces outside of the RBD bits
here...
Thanks!
sage
On Mon, 19 Sep 2011, Sage Weil wrote:
The current support for qemu and Ceph RBD (rados block device) has
two
main deficiencies: authentication doesn't work, and it relies on
environment variables (which don't work with latest upstream). This
patch set addresses both those problems.
The first two patches update the xml schemas and conf to add a Ceph
secret type and to specify authentication information along with the rbd
disk.
The next two patches make some libvirt changes. We pass virConnectPtr
down into the Domain{Attach,Detach} methods (needed to access secrets
while building the qemu command), and add a helper that will escape
arbitrary characters.
The final patch replaces the current RBD qemu code and uses the new conf
info to do authentication properly. (We still need to make a change
there to avoid having the authentication key show up on qemu command
line; I'll clean that up shortly.)
Comments on this approach?
Thanks!
sage
Changes from v1:
update docs/schemas/{domain,secret}.rng
Sage Weil (5):
secret: add Ceph secret type
storage: add authId, authDomain to virDomainDiskDef
qemu: pass virConnectPtr into Domain{Attach,Detach}*
buf: implement generic virBufferEscape
qemu/rbd: improve rbd device specification
docs/schemas/domain.rng | 6 +
docs/schemas/secret.rng | 17 ++
include/libvirt/libvirt.h.in | 3 +
src/conf/domain_conf.c | 43 +++-
src/conf/domain_conf.h | 2 +
src/conf/secret_conf.c | 45 ++++-
src/conf/secret_conf.h | 1 +
src/libvirt_private.syms | 1 +
src/qemu/qemu_command.c | 273 +++++++++++---------
src/qemu/qemu_command.h | 3 +-
src/qemu/qemu_driver.c | 17 +-
src/qemu/qemu_hotplug.c | 15 +-
src/qemu/qemu_hotplug.h | 9 +-
src/secret/secret_driver.c | 8 +
src/util/buf.c | 33 ++-
src/util/buf.h | 1 +
.../qemuxml2argv-disk-drive-network-rbd.args | 6 +-
.../qemuxml2argv-disk-drive-network-rbd.xml | 1 +
18 files changed, 328 insertions(+), 156 deletions(-)
--
1.7.4.1
>From 498cd06b76bbb4415a2f81f9d169f267ff99329c Mon Sep 17 00:00:00 2001
From: Sage Weil <sage(a)newdream.net>
Date: Thu, 15 Sep 2011 13:47:40 -0700
Subject: [PATCH 0/5] Improve Ceph Qemu+RBD support
The current support for qemu and Ceph RBD (rados block device) has two
main deficiencies: authentication doesn't work, and it relies on
environment variables (which don't work with latest upstream).
This patch set addresses both those problems, while trying to integrate as
cleanly as possible with the rest of libvirt.
The first few patches make some changes to libvirt itself: adding a CEPH
secret type (for Ceph/RBD authentication), adding authentication fields
to the XML schema, passing the virConnectPtr into the
Domain{Attach,Detach} methods (needed to access secrets while building
the qemu command), a helper that will escape arbitrary characters, and
finally a patch that replaces the current RBD qemu code.
Comments on this approach?
Thanks!
sage
Sage Weil (5):
secret: add Ceph secret type
storage: add authId, authDomain to virDomainDiskDef
qemu: pass virConnectPtr into Domain{Attach,Detach}*
buf: implement generic virBufferEscape
qemu/rbd: improve rbd device specification
include/libvirt/libvirt.h.in | 3 +
src/conf/domain_conf.c | 43 +++-
src/conf/domain_conf.h | 2 +
src/conf/secret_conf.c | 45 ++++-
src/conf/secret_conf.h | 1 +
src/libvirt_private.syms | 1 +
src/qemu/qemu_command.c | 273 +++++++++++---------
src/qemu/qemu_command.h | 3 +-
src/qemu/qemu_driver.c | 17 +-
src/qemu/qemu_hotplug.c | 15 +-
src/qemu/qemu_hotplug.h | 9 +-
src/secret/secret_driver.c | 8 +
src/util/buf.c | 33 ++-
src/util/buf.h | 1 +
.../qemuxml2argv-disk-drive-network-rbd.args | 6 +-
.../qemuxml2argv-disk-drive-network-rbd.xml | 1 +
16 files changed, 305 insertions(+), 156 deletions(-)
--
1.7.4.1
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
6:39 a.m.
Hi,
On Mon, 2011-09-19 at 21:13 -0700, Sage Weil wrote:
The current support for qemu and Ceph RBD (rados block device) has
two
main deficiencies: authentication doesn't work, and it relies on
environment variables (which don't work with latest upstream). This
patch set addresses both those problems.
Authentication is something that is really needed in the libvirt
implementation of RBD.
The interest in RBD is growing rapidly, but without auth support it's
kind of useless.
Would like to see this implemented!
Wido
The first two patches update the xml schemas and conf to add a Ceph
secret type and to specify authentication information along with the rbd
disk.
The next two patches make some libvirt changes. We pass virConnectPtr
down into the Domain{Attach,Detach} methods (needed to access secrets
while building the qemu command), and add a helper that will escape
arbitrary characters.
The final patch replaces the current RBD qemu code and uses the new conf
info to do authentication properly. (We still need to make a change
there to avoid having the authentication key show up on qemu command
line; I'll clean that up shortly.)
Comments on this approach?
Thanks!
sage
Changes from v1:
update docs/schemas/{domain,secret}.rng
Sage Weil (5):
secret: add Ceph secret type
storage: add authId, authDomain to virDomainDiskDef
qemu: pass virConnectPtr into Domain{Attach,Detach}*
buf: implement generic virBufferEscape
qemu/rbd: improve rbd device specification
docs/schemas/domain.rng | 6 +
docs/schemas/secret.rng | 17 ++
include/libvirt/libvirt.h.in | 3 +
src/conf/domain_conf.c | 43 +++-
src/conf/domain_conf.h | 2 +
src/conf/secret_conf.c | 45 ++++-
src/conf/secret_conf.h | 1 +
src/libvirt_private.syms | 1 +
src/qemu/qemu_command.c | 273 +++++++++++---------
src/qemu/qemu_command.h | 3 +-
src/qemu/qemu_driver.c | 17 +-
src/qemu/qemu_hotplug.c | 15 +-
src/qemu/qemu_hotplug.h | 9 +-
src/secret/secret_driver.c | 8 +
src/util/buf.c | 33 ++-
src/util/buf.h | 1 +
.../qemuxml2argv-disk-drive-network-rbd.args | 6 +-
.../qemuxml2argv-disk-drive-network-rbd.xml | 1 +
18 files changed, 328 insertions(+), 156 deletions(-)
--
1.7.4.1
>From 498cd06b76bbb4415a2f81f9d169f267ff99329c Mon Sep 17 00:00:00 2001
From: Sage Weil <sage(a)newdream.net>
Date: Thu, 15 Sep 2011 13:47:40 -0700
Subject: [PATCH 0/5] Improve Ceph Qemu+RBD support
The current support for qemu and Ceph RBD (rados block device) has two
main deficiencies: authentication doesn't work, and it relies on
environment variables (which don't work with latest upstream).
This patch set addresses both those problems, while trying to integrate as
cleanly as possible with the rest of libvirt.
The first few patches make some changes to libvirt itself: adding a CEPH
secret type (for Ceph/RBD authentication), adding authentication fields
to the XML schema, passing the virConnectPtr into the
Domain{Attach,Detach} methods (needed to access secrets while building
the qemu command), a helper that will escape arbitrary characters, and
finally a patch that replaces the current RBD qemu code.
Comments on this approach?
Thanks!
sage
Sage Weil (5):
secret: add Ceph secret type
storage: add authId, authDomain to virDomainDiskDef
qemu: pass virConnectPtr into Domain{Attach,Detach}*
buf: implement generic virBufferEscape
qemu/rbd: improve rbd device specification
include/libvirt/libvirt.h.in | 3 +
src/conf/domain_conf.c | 43 +++-
src/conf/domain_conf.h | 2 +
src/conf/secret_conf.c | 45 ++++-
src/conf/secret_conf.h | 1 +
src/libvirt_private.syms | 1 +
src/qemu/qemu_command.c | 273 +++++++++++---------
src/qemu/qemu_command.h | 3 +-
src/qemu/qemu_driver.c | 17 +-
src/qemu/qemu_hotplug.c | 15 +-
src/qemu/qemu_hotplug.h | 9 +-
src/secret/secret_driver.c | 8 +
src/util/buf.c | 33 ++-
src/util/buf.h | 1 +
.../qemuxml2argv-disk-drive-network-rbd.args | 6 +-
.../qemuxml2argv-disk-drive-network-rbd.xml | 1 +
16 files changed, 305 insertions(+), 156 deletions(-)
7:17 a.m.
On Fri, Oct 07, 2011 at 01:39:50PM +0200, Wido den Hollander wrote:
Hi,
On Mon, 2011-09-19 at 21:13 -0700, Sage Weil wrote:
> The current support for qemu and Ceph RBD (rados block device) has two
> main deficiencies: authentication doesn't work, and it relies on
> environment variables (which don't work with latest upstream). This
> patch set addresses both those problems.
>
Authentication is something that is really needed in the libvirt
implementation of RBD.
The interest in RBD is growing rapidly, but without auth support it's
kind of useless.
Would like to see this implemented!
Apologies for not reviewing this patch series before now. It accidentally
slipped off my todo list. I'll aim to take a look at in at the start
of next week
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
11:33 a.m.
On Mon, Sep 19, 2011 at 09:13:38PM -0700, Sage Weil wrote:
The current support for qemu and Ceph RBD (rados block device) has
two
main deficiencies: authentication doesn't work, and it relies on
environment variables (which don't work with latest upstream). This
patch set addresses both those problems.
The first two patches update the xml schemas and conf to add a Ceph
secret type and to specify authentication information along with the rbd
disk.
The next two patches make some libvirt changes. We pass virConnectPtr
down into the Domain{Attach,Detach} methods (needed to access secrets
while building the qemu command), and add a helper that will escape
arbitrary characters.
The final patch replaces the current RBD qemu code and uses the new conf
info to do authentication properly. (We still need to make a change
there to avoid having the authentication key show up on qemu command
line; I'll clean that up shortly.)
Comments on this approach?
Ok, I've finally got myself a Ceph cluster up & running, with RBD
exports to a QEMU guest[1], so I can give more sensible answers to
these patches :-)
Overall you are taking the current XML for a disk:
<disk type='network' device='disk'>
<driver name='qemu' type='raw'/>
<source protocol='rbd' name='demo/wibble'>
<host name='lettuce.example.org' port='6798'/>
<host name='mustard.example.org' port='6798'/>
<host name='avocado.example.org' port='6798'/>
</source>
<target dev='vdb' bus='virtio'/>
</disk>
and adding one new element <auth> such that we get
<disk type='network' device='disk'>
<driver name='qemu' type='raw'/>
<source protocol='rbd' name='demo/wibble'>
<auth id='admin' domain='clustername'/>
<host name='lettuce.example.org' port='6798'/>
<host name='mustard.example.org' port='6798'/>
<host name='avocado.example.org' port='6798'/>
</source>
<target dev='vdb' bus='virtio'/>
</disk>
together with some secret XML that looks like:
<secret ephemeral='no' private='no'>
<uuid>0a81f5b2-8403-7b23-c8d6-21ccc2f80d6f</uuid>
<usage type='ceph'>
<auth id='admin' domain='clustername'/>
</usage>
</secret>
When starting a guest the auth id + domain are concatenated to find
the secret. The 'id' is also used as the username to pass to QEMU.
The 'domain' is only ever used for the secret lookup part, never
passed to QEMU, since the host IP addrs do that job.
By comparison, QCow2 encryption disks are done with:
<disk type='file' device='disk'>
<driver name='qemu' type='qcow2'/>
<source file='/home/berrange/VirtualMachines/demo.qcow2'/>
<target dev='hda' bus='ide'/>
<encryption format='qcow'>
<secret type='passphrase'
uuid='0a81f5b2-8403-7b23-c8d6-21ccc2f80d6f'/>
</encryption>
</disk>
And the secret with:
<secret ephemeral='no' private='no'>
<uuid>0a81f5b2-8403-7b23-c8d6-21ccc2f80d6f</uuid>
<usage type='volume'>
<volume>/home/berrange/VirtualMachines/demo.qcow2</volume>
</usage>
</secret>
When starting a guest, the secret UUID is used to find the passphrase.
So we have a difference in the way the secrets are linked to the disks
between Ceph and QCow2 here.
I can see the appeal of doing it the way you have, but at the same time
I would like to have consistency with the QCow2 approach, and UUIDs are
a stronger identifier IMHO.
Also, although the secret XML is exposed auth id + domain as separate
attributes in the XML, internally they're processed as a concatenated
string, likewise the public API uses a concatenated string form. So
I think the XML ought to use that form too, and not split them.
Finally, I think the concept of authentication credentials for
disks, could be more generally useful than just for Ceph or other
network block devs, so I'm somewhat inclined to move it outside the
<source> element
Thus I would propose a slight variation on what you've done:
<disk type='network' device='disk'>
<driver name='qemu' type='raw'/>
<auth username='admin'>
<secret type='passphrase'
uuid='0a81f5b2-8403-7b23-c8d6-21ccc2f80d6f'/>
</auth>
<source protocol='rbd' name='demo/wibble'>
<host name='lettuce.example.org' port='6798'/>
<host name='mustard.example.org' port='6798'/>
<host name='avocado.example.org' port='6798'/>
</source>
<target dev='vdb' bus='virtio'/>
</disk>
And in the secret XML:
<secret ephemeral='no' private='no'>
<uuid>0a81f5b2-8403-7b23-c8d6-21ccc2f80d6f</uuid>
<usage type='ceph'>
<domain>some.cluster.name/admin</domain>
</usage>
</secret>
Though I think the UUID based lookup should be primary, I would also
accept an optional alternate syntax based on usage strings, if you
think that's important:
<disk type='network' device='disk'>
<driver name='qemu' type='raw'/>
<auth username='admin'>
<secret type='passphrase' usage='some.cluster.name/admin'/>
</auth>
<source protocol='rbd' name='demo/wibble'>
<host name='lettuce.example.org' port='6798'/>
<host name='mustard.example.org' port='6798'/>
<host name='avocado.example.org' port='6798'/>
</source>
<target dev='vdb' bus='virtio'/>
</disk>
Regards,
Daniel
[1]
http://berrange.com/posts/2011/10/12/setting-up-a-ceph-cluster-and-export...
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
11:45 a.m.
Hi Daniel,
On Wed, 12 Oct 2011, Daniel P. Berrange wrote:
On Mon, Sep 19, 2011 at 09:13:38PM -0700, Sage Weil wrote:
> The current support for qemu and Ceph RBD (rados block device) has two
> main deficiencies: authentication doesn't work, and it relies on
> environment variables (which don't work with latest upstream). This
> patch set addresses both those problems.
>
> The first two patches update the xml schemas and conf to add a Ceph
> secret type and to specify authentication information along with the rbd
> disk.
>
> The next two patches make some libvirt changes. We pass virConnectPtr
> down into the Domain{Attach,Detach} methods (needed to access secrets
> while building the qemu command), and add a helper that will escape
> arbitrary characters.
>
> The final patch replaces the current RBD qemu code and uses the new conf
> info to do authentication properly. (We still need to make a change
> there to avoid having the authentication key show up on qemu command
> line; I'll clean that up shortly.)
>
> Comments on this approach?
Ok, I've finally got myself a Ceph cluster up & running, with RBD
exports to a QEMU guest[1], so I can give more sensible answers to
these patches :-)
Overall you are taking the current XML for a disk:
<disk type='network' device='disk'>
<driver name='qemu' type='raw'/>
<source protocol='rbd' name='demo/wibble'>
<host name='lettuce.example.org' port='6798'/>
<host name='mustard.example.org' port='6798'/>
<host name='avocado.example.org' port='6798'/>
</source>
<target dev='vdb' bus='virtio'/>
</disk>
and adding one new element <auth> such that we get
<disk type='network' device='disk'>
<driver name='qemu' type='raw'/>
<source protocol='rbd' name='demo/wibble'>
<auth id='admin' domain='clustername'/>
<host name='lettuce.example.org' port='6798'/>
<host name='mustard.example.org' port='6798'/>
<host name='avocado.example.org' port='6798'/>
</source>
<target dev='vdb' bus='virtio'/>
</disk>
together with some secret XML that looks like:
<secret ephemeral='no' private='no'>
<uuid>0a81f5b2-8403-7b23-c8d6-21ccc2f80d6f</uuid>
<usage type='ceph'>
<auth id='admin' domain='clustername'/>
</usage>
</secret>
When starting a guest the auth id + domain are concatenated to find
the secret. The 'id' is also used as the username to pass to QEMU.
The 'domain' is only ever used for the secret lookup part, never
passed to QEMU, since the host IP addrs do that job.
By comparison, QCow2 encryption disks are done with:
<disk type='file' device='disk'>
<driver name='qemu' type='qcow2'/>
<source file='/home/berrange/VirtualMachines/demo.qcow2'/>
<target dev='hda' bus='ide'/>
<encryption format='qcow'>
<secret type='passphrase'
uuid='0a81f5b2-8403-7b23-c8d6-21ccc2f80d6f'/>
</encryption>
</disk>
And the secret with:
<secret ephemeral='no' private='no'>
<uuid>0a81f5b2-8403-7b23-c8d6-21ccc2f80d6f</uuid>
<usage type='volume'>
<volume>/home/berrange/VirtualMachines/demo.qcow2</volume>
</usage>
</secret>
When starting a guest, the secret UUID is used to find the passphrase.
So we have a difference in the way the secrets are linked to the disks
between Ceph and QCow2 here.
I can see the appeal of doing it the way you have, but at the same time
I would like to have consistency with the QCow2 approach, and UUIDs are
a stronger identifier IMHO.
Also, although the secret XML is exposed auth id + domain as separate
attributes in the XML, internally they're processed as a concatenated
string, likewise the public API uses a concatenated string form. So
I think the XML ought to use that form too, and not split them.
Finally, I think the concept of authentication credentials for
disks, could be more generally useful than just for Ceph or other
network block devs, so I'm somewhat inclined to move it outside the
<source> element
Thus I would propose a slight variation on what you've done:
<disk type='network' device='disk'>
<driver name='qemu' type='raw'/>
<auth username='admin'>
<secret type='passphrase'
uuid='0a81f5b2-8403-7b23-c8d6-21ccc2f80d6f'/>
</auth>
<source protocol='rbd' name='demo/wibble'>
<host name='lettuce.example.org' port='6798'/>
<host name='mustard.example.org' port='6798'/>
<host name='avocado.example.org' port='6798'/>
</source>
<target dev='vdb' bus='virtio'/>
</disk>
And in the secret XML:
<secret ephemeral='no' private='no'>
<uuid>0a81f5b2-8403-7b23-c8d6-21ccc2f80d6f</uuid>
<usage type='ceph'>
<domain>some.cluster.name/admin</domain>
</usage>
</secret>
Though I think the UUID based lookup should be primary, I would also
accept an optional alternate syntax based on usage strings, if you
think that's important:
<disk type='network' device='disk'>
<driver name='qemu' type='raw'/>
<auth username='admin'>
<secret type='passphrase' usage='some.cluster.name/admin'/>
</auth>
<source protocol='rbd' name='demo/wibble'>
<host name='lettuce.example.org' port='6798'/>
<host name='mustard.example.org' port='6798'/>
<host name='avocado.example.org' port='6798'/>
</source>
<target dev='vdb' bus='virtio'/>
</disk>
Yes, I like this much better as well. I wasn't particularly happy with
how the keys were being identified; this is much cleaner.
I'll respin these and resend shortly.
Thanks for the review!
sage
4790
days inactive
4812
days old
16 comments
4 participants
participants (4)
-
Daniel P. Berrange -
Eric Blake -
Sage Weil -
Wido den Hollander