* [PATCH 0/5] Improve Ceph Qemu+RBD support
@ 2011-09-15 20:52 Sage Weil
2011-09-15 20:52 ` [PATCH 1/5] secret: add Ceph secret type Sage Weil
` (4 more replies)
0 siblings, 5 replies; 10+ messages in thread
From: Sage Weil @ 2011-09-15 20:52 UTC (permalink / raw)
To: libvir-list; +Cc: ceph-devel, Sage Weil
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/5] secret: add Ceph secret type
2011-09-15 20:52 [PATCH 0/5] Improve Ceph Qemu+RBD support Sage Weil
@ 2011-09-15 20:52 ` Sage Weil
2011-09-16 8:18 ` [libvirt] " Osier Yang
2011-09-15 20:52 ` [PATCH 2/5] storage: add authId, authDomain to virDomainDiskDef Sage Weil
` (3 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Sage Weil @ 2011-09-15 20:52 UTC (permalink / raw)
To: libvir-list; +Cc: ceph-devel, Sage Weil
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@newdream.net>
---
include/libvirt/libvirt.h.in | 3 ++
src/conf/secret_conf.c | 45 +++++++++++++++++++++++++++++++++++++++++-
src/conf/secret_conf.h | 1 +
src/secret/secret_driver.c | 8 +++++++
4 files changed, 56 insertions(+), 1 deletions(-)
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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/5] storage: add authId, authDomain to virDomainDiskDef
2011-09-15 20:52 [PATCH 0/5] Improve Ceph Qemu+RBD support Sage Weil
2011-09-15 20:52 ` [PATCH 1/5] secret: add Ceph secret type Sage Weil
@ 2011-09-15 20:52 ` Sage Weil
2011-09-15 20:52 ` [PATCH 3/5] qemu: pass virConnectPtr into Domain{Attach,Detach}* Sage Weil
` (2 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: Sage Weil @ 2011-09-15 20:52 UTC (permalink / raw)
To: libvir-list; +Cc: ceph-devel, Sage Weil
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@newdream.net>
---
src/conf/domain_conf.c | 43 ++++++++++++++++++++++++++++++++++---------
src/conf/domain_conf.h | 2 ++
2 files changed, 36 insertions(+), 9 deletions(-)
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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/5] qemu: pass virConnectPtr into Domain{Attach,Detach}*
2011-09-15 20:52 [PATCH 0/5] Improve Ceph Qemu+RBD support Sage Weil
2011-09-15 20:52 ` [PATCH 1/5] secret: add Ceph secret type Sage Weil
2011-09-15 20:52 ` [PATCH 2/5] storage: add authId, authDomain to virDomainDiskDef Sage Weil
@ 2011-09-15 20:52 ` Sage Weil
2011-09-15 20:52 ` [PATCH 4/5] buf: implement generic virBufferEscape Sage Weil
2011-09-15 20:52 ` [PATCH 5/5] qemu/rbd: improve rbd device specification Sage Weil
4 siblings, 0 replies; 10+ messages in thread
From: Sage Weil @ 2011-09-15 20:52 UTC (permalink / raw)
To: libvir-list; +Cc: ceph-devel, Sage Weil
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@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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 4/5] buf: implement generic virBufferEscape
2011-09-15 20:52 [PATCH 0/5] Improve Ceph Qemu+RBD support Sage Weil
` (2 preceding siblings ...)
2011-09-15 20:52 ` [PATCH 3/5] qemu: pass virConnectPtr into Domain{Attach,Detach}* Sage Weil
@ 2011-09-15 20:52 ` Sage Weil
2011-09-15 20:52 ` [PATCH 5/5] qemu/rbd: improve rbd device specification Sage Weil
4 siblings, 0 replies; 10+ messages in thread
From: Sage Weil @ 2011-09-15 20:52 UTC (permalink / raw)
To: libvir-list; +Cc: ceph-devel, Sage Weil
Implement a generic helper to escape a given set of characters with a
leading '\'. Generalizes virBufferEscapeSexpr().
Signed-off-by: Sage Weil <sage@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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 5/5] qemu/rbd: improve rbd device specification
2011-09-15 20:52 [PATCH 0/5] Improve Ceph Qemu+RBD support Sage Weil
` (3 preceding siblings ...)
2011-09-15 20:52 ` [PATCH 4/5] buf: implement generic virBufferEscape Sage Weil
@ 2011-09-15 20:52 ` Sage Weil
2011-09-16 16:52 ` Tommi Virtanen
4 siblings, 1 reply; 10+ messages in thread
From: Sage Weil @ 2011-09-15 20:52 UTC (permalink / raw)
To: libvir-list; +Cc: ceph-devel, Sage Weil
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@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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [libvirt] [PATCH 1/5] secret: add Ceph secret type
2011-09-15 20:52 ` [PATCH 1/5] secret: add Ceph secret type Sage Weil
@ 2011-09-16 8:18 ` Osier Yang
0 siblings, 0 replies; 10+ messages in thread
From: Osier Yang @ 2011-09-16 8:18 UTC (permalink / raw)
To: Sage Weil; +Cc: libvir-list, ceph-devel
于 2011年09月16日 04:52, Sage Weil 写道:
> 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@newdream.net>
> ---
> include/libvirt/libvirt.h.in | 3 ++
> src/conf/secret_conf.c | 45 +++++++++++++++++++++++++++++++++++++++++-
> src/conf/secret_conf.h | 1 +
> src/secret/secret_driver.c | 8 +++++++
> 4 files changed, 56 insertions(+), 1 deletions(-)
>
> 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);
Should use VIR_ALLOC/VIR_ALLOC_N here.
> + 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;
> }
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 5/5] qemu/rbd: improve rbd device specification
2011-09-15 20:52 ` [PATCH 5/5] qemu/rbd: improve rbd device specification Sage Weil
@ 2011-09-16 16:52 ` Tommi Virtanen
2011-09-16 17:01 ` Sage Weil
0 siblings, 1 reply; 10+ messages in thread
From: Tommi Virtanen @ 2011-09-16 16:52 UTC (permalink / raw)
To: Sage Weil; +Cc: libvir-list, ceph-devel
On Thu, Sep 15, 2011 at 13:52, Sage Weil <sage@newdream.net> wrote:
> +static int buildRBDString(virConnectPtr conn,
...
> + /* look up secret */
> + snprintf(idDomain, sizeof(idDomain), "%s/%s", disk->authId,
> + disk->authDomain);
> + sec = virSecretLookupByUsage(conn,
> + VIR_SECRET_USAGE_TYPE_CEPH,
> + idDomain);
...
> + 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);
If I'm reading this right, that puts the ceph secret on the kvm
command line. That's not good, that makes it visible to anyone on the
host.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 5/5] qemu/rbd: improve rbd device specification
2011-09-16 16:52 ` Tommi Virtanen
@ 2011-09-16 17:01 ` Sage Weil
2011-09-16 17:11 ` Tommi Virtanen
0 siblings, 1 reply; 10+ messages in thread
From: Sage Weil @ 2011-09-16 17:01 UTC (permalink / raw)
To: Tommi Virtanen; +Cc: libvir-list, ceph-devel
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1294 bytes --]
On Fri, 16 Sep 2011, Tommi Virtanen wrote:
> On Thu, Sep 15, 2011 at 13:52, Sage Weil <sage@newdream.net> wrote:
> > +static int buildRBDString(virConnectPtr conn,
> ...
> > + /* look up secret */
> > + snprintf(idDomain, sizeof(idDomain), "%s/%s", disk->authId,
> > + disk->authDomain);
> > + sec = virSecretLookupByUsage(conn,
> > + VIR_SECRET_USAGE_TYPE_CEPH,
> > + idDomain);
> ...
> > + 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);
>
> If I'm reading this right, that puts the ceph secret on the kvm
> command line. That's not good, that makes it visible to anyone on the
> host.
Yeah, we definitely want something better, but I wanted to make sure the
overall approach is fine before doing something too annoying with
temporary unlinked files or environment variables or something.
sage
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 5/5] qemu/rbd: improve rbd device specification
2011-09-16 17:01 ` Sage Weil
@ 2011-09-16 17:11 ` Tommi Virtanen
0 siblings, 0 replies; 10+ messages in thread
From: Tommi Virtanen @ 2011-09-16 17:11 UTC (permalink / raw)
To: Sage Weil; +Cc: libvir-list, ceph-devel
On Fri, Sep 16, 2011 at 10:01, Sage Weil <sage@newdream.net> wrote:
>> If I'm reading this right, that puts the ceph secret on the kvm
>> command line. That's not good, that makes it visible to anyone on the
>> host.
> Yeah, we definitely want something better, but I wanted to make sure the
> overall approach is fine before doing something too annoying with
> temporary unlinked files or environment variables or something.
That sounds good. Except for the environment variables part; while
some effort is put into guarding the environment, there have been
several ways of reading other processes' environment, historically. I
wouldn't rely on environment to stay secret, ever.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2011-09-16 17:12 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-09-15 20:52 [PATCH 0/5] Improve Ceph Qemu+RBD support Sage Weil
2011-09-15 20:52 ` [PATCH 1/5] secret: add Ceph secret type Sage Weil
2011-09-16 8:18 ` [libvirt] " Osier Yang
2011-09-15 20:52 ` [PATCH 2/5] storage: add authId, authDomain to virDomainDiskDef Sage Weil
2011-09-15 20:52 ` [PATCH 3/5] qemu: pass virConnectPtr into Domain{Attach,Detach}* Sage Weil
2011-09-15 20:52 ` [PATCH 4/5] buf: implement generic virBufferEscape Sage Weil
2011-09-15 20:52 ` [PATCH 5/5] qemu/rbd: improve rbd device specification Sage Weil
2011-09-16 16:52 ` Tommi Virtanen
2011-09-16 17:01 ` Sage Weil
2011-09-16 17:11 ` Tommi Virtanen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.