5:13 a.m.
Since there is a shmobj leak when let qemu create shmobj by
themselves, also the label of shmobj/shmem-server socket
is not right. Guest cannot direct use the shmem-server
if users enabled selinux. So it will be better to manage it
in libvirt.
The way i chosed is region the shmem deivce in a list, and
save it status to a local file to avoid losing it after restart
libvirtd, and count the guest which use it, and let the callers
know if there is no guest is using it (then we can relabel/cleanup
some resource).
Notice: you still cannot use the ivshmem-server if the process label
is not correct, just set the socket label is not enought, selinux
still will forbid qemu use it, because the shmem-server's process is
not correct, you will find the AVC like this (i set up the ivshmem
server via shell):
type=AVC msg=audit(1437642157.227:73784): avc: denied { connectto } for \
pid=6137 comm="qemu-kvm" path="/tmp/ivshmem_socket" \
scontext=system_u:system_r:svirt_t:s0:c703,c707 \
tcontext=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 tclass=unix_stream_socket
But the problem is we cannot change the running shm-server process label,
We need wait ivshmem-server to be a part of qemu progrem, then setup the
ivshmem-server by libvirt. we cannot do nothing for the ivshmem-server right now.
Luyao Huang (4):
conf: introduce seclabels in shmem device element
security: add security part for shmem device
util: introduce new helpers to manage shmem device
qemu: call the helpers in virshm.c to manage shmem device
configure.ac | 10 +
docs/formatdomain.html.in | 7 +
docs/schemas/domaincommon.rng | 3 +
po/POTFILES.in | 3 +-
src/Makefile.am | 5 +-
src/conf/domain_conf.c | 55 +++-
src/conf/domain_conf.h | 5 +
src/libvirt_private.syms | 18 ++
src/qemu/qemu_conf.h | 3 +
src/qemu/qemu_driver.c | 4 +
src/qemu/qemu_process.c | 158 ++++++++++
src/security/security_dac.c | 67 +++++
src/security/security_driver.h | 11 +
src/security/security_manager.c | 38 +++
src/security/security_manager.h | 8 +
src/security/security_selinux.c | 70 +++++
src/security/security_stack.c | 41 +++
src/util/virshm.c | 623 ++++++++++++++++++++++++++++++++++++++++
src/util/virshm.h | 104 +++++++
19 files changed, 1220 insertions(+), 13 deletions(-)
create mode 100644 src/util/virshm.c
create mode 100644 src/util/virshm.h
--
1.8.3.1
5:13 a.m.
New subject: [libvirt] [PATCH 1/4] conf: introduce seclabels in shmem device element
Introduce a new element in shmem device element, this
could help users to change the shm label to a specified
label.
Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
---
docs/formatdomain.html.in | 7 ++++++
docs/schemas/domaincommon.rng | 3 +++
src/conf/domain_conf.c | 55 ++++++++++++++++++++++++++++++++++---------
src/conf/domain_conf.h | 5 ++++
4 files changed, 59 insertions(+), 11 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index d0c1741..e02c67c 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -6098,6 +6098,13 @@ qemu-kvm -net nic,model=? /dev/null
vectors. The <code>ioeventd</code> attribute enables/disables (values
"on"/"off", respectively) ioeventfd.
</dd>
+ <dt><code>seclabel</code></dt>
+ <dd>
+ The optional <code>seclabel</code> to override the way that labelling
+ is done on the shm object path or shm server path. If this
+ element is not present, the <a href="#seclabel">security label is
inherited
+ from the per-domain setting</a>.
+ </dd>
</dl>
<h4><a name="elementsMemory">Memory
devices</a></h4>
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 1120003..f58e8de 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3323,6 +3323,9 @@
</optional>
</element>
</optional>
+ <zeroOrMore>
+ <ref name='devSeclabel'/>
+ </zeroOrMore>
<optional>
<ref name="address"/>
</optional>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 73ac537..cb3d72a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11261,6 +11261,8 @@ virDomainNVRAMDefParseXML(xmlNodePtr node,
static virDomainShmemDefPtr
virDomainShmemDefParseXML(xmlNodePtr node,
xmlXPathContextPtr ctxt,
+ virSecurityLabelDefPtr* vmSeclabels,
+ int nvmSeclabels,
unsigned int flags)
{
char *tmp = NULL;
@@ -11332,6 +11334,10 @@ virDomainShmemDefParseXML(xmlNodePtr node,
if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0)
goto cleanup;
+ if (virSecurityDeviceLabelDefParseXML(&def->seclabels,
&def->nseclabels,
+ vmSeclabels, nvmSeclabels,
+ ctxt, flags) < 0)
+ goto cleanup;
ret = def;
def = NULL;
@@ -12457,7 +12463,11 @@ virDomainDeviceDefParse(const char *xmlStr,
goto error;
break;
case VIR_DOMAIN_DEVICE_SHMEM:
- if (!(dev->data.shmem = virDomainShmemDefParseXML(node, ctxt, flags)))
+ if (!(dev->data.shmem = virDomainShmemDefParseXML(node,
+ ctxt,
+ def->seclabels,
+ def->nseclabels,
+ flags)))
goto error;
break;
case VIR_DOMAIN_DEVICE_TPM:
@@ -16136,7 +16146,8 @@ virDomainDefParseXML(xmlDocPtr xml,
for (i = 0; i < n; i++) {
virDomainShmemDefPtr shmem;
ctxt->node = nodes[i];
- shmem = virDomainShmemDefParseXML(nodes[i], ctxt, flags);
+ shmem = virDomainShmemDefParseXML(nodes[i], ctxt, def->seclabels,
+ def->nseclabels, flags);
if (!shmem)
goto error;
@@ -20308,6 +20319,8 @@ virDomainShmemDefFormat(virBufferPtr buf,
virDomainShmemDefPtr def,
unsigned int flags)
{
+ size_t n;
+
virBufferEscapeString(buf, "<shmem name='%s'", def->name);
if (!def->size &&
@@ -20341,6 +20354,9 @@ virDomainShmemDefFormat(virBufferPtr buf,
virBufferAddLit(buf, "/>\n");
}
+ for (n = 0; n < def->nseclabels; n++)
+ virSecurityDeviceLabelDefFormat(buf, def->seclabels[n], flags);
+
if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
return -1;
@@ -23851,11 +23867,25 @@ virDomainObjListExport(virDomainObjListPtr domlist,
}
+static virSecurityDeviceLabelDefPtr
+virDomainGetDeviceSecurityLabelDef(virSecurityDeviceLabelDefPtr *seclabels,
+ size_t nseclabels,
+ const char *model)
+{
+ size_t i;
+
+ for (i = 0; i < nseclabels; i++) {
+ if (STREQ_NULLABLE(seclabels[i]->model, model))
+ return seclabels[i];
+ }
+ return NULL;
+}
+
+
virSecurityLabelDefPtr
virDomainDefGetSecurityLabelDef(virDomainDefPtr def, const char *model)
{
size_t i;
- virSecurityLabelDefPtr seclabel = NULL;
if (def == NULL || model == NULL)
return NULL;
@@ -23866,24 +23896,27 @@ virDomainDefGetSecurityLabelDef(virDomainDefPtr def, const char
*model)
if (STREQ(def->seclabels[i]->model, model))
return def->seclabels[i];
}
-
- return seclabel;
+ return NULL;
}
virSecurityDeviceLabelDefPtr
virDomainChrDefGetSecurityLabelDef(virDomainChrDefPtr def, const char *model)
{
- size_t i;
+ if (def == NULL)
+ return NULL;
+
+ return virDomainGetDeviceSecurityLabelDef(def->seclabels, def->nseclabels,
model);
+}
+
+virSecurityDeviceLabelDefPtr
+virDomainShmemDefGetSecurityLabelDef(virDomainShmemDefPtr def, const char *model)
+{
if (def == NULL)
return NULL;
- for (i = 0; i < def->nseclabels; i++) {
- if (STREQ_NULLABLE(def->seclabels[i]->model, model))
- return def->seclabels[i];
- }
- return NULL;
+ return virDomainGetDeviceSecurityLabelDef(def->seclabels, def->nseclabels,
model);
}
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 0fe6b1a..1a0475e 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1608,6 +1608,8 @@ struct _virDomainShmemDef {
unsigned vectors;
virTristateSwitch ioeventfd;
} msi;
+ size_t nseclabels;
+ virSecurityDeviceLabelDefPtr *seclabels;
virDomainDeviceInfo info;
};
@@ -2943,6 +2945,9 @@ virDomainDefGetSecurityLabelDef(virDomainDefPtr def, const char
*model);
virSecurityDeviceLabelDefPtr
virDomainChrDefGetSecurityLabelDef(virDomainChrDefPtr def, const char *model);
+virSecurityDeviceLabelDefPtr
+virDomainShmemDefGetSecurityLabelDef(virDomainShmemDefPtr def, const char *model);
+
typedef const char* (*virEventActionToStringFunc)(int type);
typedef int (*virEventActionFromStringFunc)(const char *type);
--
1.8.3.1
10:42 a.m.
New subject: [libvirt] [PATCH 1/4] conf: introduce seclabels in shmem device element
Hi
On Thu, Jul 23, 2015 at 12:13 PM, Luyao Huang <lhuang(a)redhat.com> wrote:
Introduce a new element in shmem device element, this
could help users to change the shm label to a specified
label.
Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
---
docs/formatdomain.html.in | 7 ++++++
docs/schemas/domaincommon.rng | 3 +++
src/conf/domain_conf.c | 55 ++++++++++++++++++++++++++++++++++---------
src/conf/domain_conf.h | 5 ++++
4 files changed, 59 insertions(+), 11 deletions(-)
It would be better with a small test, checking parsing and format.
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index d0c1741..e02c67c 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -6098,6 +6098,13 @@ qemu-kvm -net nic,model=? /dev/null
vectors. The <code>ioeventd</code> attribute enables/disables (values
"on"/"off", respectively) ioeventfd.
</dd>
+ <dt><code>seclabel</code></dt>
+ <dd>
+ The optional <code>seclabel</code> to override the way that labelling
The "element may contain an" optional <code>...
+ is done on the shm object path or shm server path. If this
+ element is not present, the <a href="#seclabel">security label is
inherited
+ from the per-domain setting</a>.
+ </dd>
</dl>
<h4><a name="elementsMemory">Memory
devices</a></h4>
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 1120003..f58e8de 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3323,6 +3323,9 @@
</optional>
</element>
</optional>
+ <zeroOrMore>
+ <ref name='devSeclabel'/>
+ </zeroOrMore>
<optional>
<ref name="address"/>
</optional>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 73ac537..cb3d72a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11261,6 +11261,8 @@ virDomainNVRAMDefParseXML(xmlNodePtr node,
static virDomainShmemDefPtr
virDomainShmemDefParseXML(xmlNodePtr node,
xmlXPathContextPtr ctxt,
+ virSecurityLabelDefPtr* vmSeclabels,
+ int nvmSeclabels,
unsigned int flags)
{
char *tmp = NULL;
@@ -11332,6 +11334,10 @@ virDomainShmemDefParseXML(xmlNodePtr node,
if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0)
goto cleanup;
+ if (virSecurityDeviceLabelDefParseXML(&def->seclabels,
&def->nseclabels,
+ vmSeclabels, nvmSeclabels,
+ ctxt, flags) < 0)
+ goto cleanup;
ret = def;
def = NULL;
@@ -12457,7 +12463,11 @@ virDomainDeviceDefParse(const char *xmlStr,
goto error;
break;
case VIR_DOMAIN_DEVICE_SHMEM:
- if (!(dev->data.shmem = virDomainShmemDefParseXML(node, ctxt, flags)))
+ if (!(dev->data.shmem = virDomainShmemDefParseXML(node,
+ ctxt,
+ def->seclabels,
+ def->nseclabels,
+ flags)))
goto error;
break;
case VIR_DOMAIN_DEVICE_TPM:
@@ -16136,7 +16146,8 @@ virDomainDefParseXML(xmlDocPtr xml,
for (i = 0; i < n; i++) {
virDomainShmemDefPtr shmem;
ctxt->node = nodes[i];
- shmem = virDomainShmemDefParseXML(nodes[i], ctxt, flags);
+ shmem = virDomainShmemDefParseXML(nodes[i], ctxt, def->seclabels,
+ def->nseclabels, flags);
if (!shmem)
goto error;
@@ -20308,6 +20319,8 @@ virDomainShmemDefFormat(virBufferPtr buf,
virDomainShmemDefPtr def,
unsigned int flags)
{
+ size_t n;
+
virBufferEscapeString(buf, "<shmem name='%s'", def->name);
if (!def->size &&
@@ -20341,6 +20354,9 @@ virDomainShmemDefFormat(virBufferPtr buf,
virBufferAddLit(buf, "/>\n");
}
+ for (n = 0; n < def->nseclabels; n++)
+ virSecurityDeviceLabelDefFormat(buf, def->seclabels[n], flags);
+
if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
return -1;
@@ -23851,11 +23867,25 @@ virDomainObjListExport(virDomainObjListPtr domlist,
}
+static virSecurityDeviceLabelDefPtr
+virDomainGetDeviceSecurityLabelDef(virSecurityDeviceLabelDefPtr *seclabels,
+ size_t nseclabels,
+ const char *model)
+{
+ size_t i;
+
+ for (i = 0; i < nseclabels; i++) {
+ if (STREQ_NULLABLE(seclabels[i]->model, model))
+ return seclabels[i];
+ }
+ return NULL;
+}
+
+
virSecurityLabelDefPtr
virDomainDefGetSecurityLabelDef(virDomainDefPtr def, const char *model)
{
size_t i;
- virSecurityLabelDefPtr seclabel = NULL;
if (def == NULL || model == NULL)
return NULL;
@@ -23866,24 +23896,27 @@ virDomainDefGetSecurityLabelDef(virDomainDefPtr def, const char
*model)
if (STREQ(def->seclabels[i]->model, model))
return def->seclabels[i];
}
-
- return seclabel;
+ return NULL;
This looks like a seperate cleanup.
}
virSecurityDeviceLabelDefPtr
virDomainChrDefGetSecurityLabelDef(virDomainChrDefPtr def, const char *model)
{
- size_t i;
+ if (def == NULL)
+ return NULL;
+
+ return virDomainGetDeviceSecurityLabelDef(def->seclabels, def->nseclabels,
model);
+}
+
+virSecurityDeviceLabelDefPtr
+virDomainShmemDefGetSecurityLabelDef(virDomainShmemDefPtr def, const char *model)
+{
if (def == NULL)
return NULL;
- for (i = 0; i < def->nseclabels; i++) {
- if (STREQ_NULLABLE(def->seclabels[i]->model, model))
- return def->seclabels[i];
- }
- return NULL;
+ return virDomainGetDeviceSecurityLabelDef(def->seclabels, def->nseclabels,
model);
}
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 0fe6b1a..1a0475e 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1608,6 +1608,8 @@ struct _virDomainShmemDef {
unsigned vectors;
virTristateSwitch ioeventfd;
} msi;
+ size_t nseclabels;
+ virSecurityDeviceLabelDefPtr *seclabels;
virDomainDeviceInfo info;
};
@@ -2943,6 +2945,9 @@ virDomainDefGetSecurityLabelDef(virDomainDefPtr def, const char
*model);
virSecurityDeviceLabelDefPtr
virDomainChrDefGetSecurityLabelDef(virDomainChrDefPtr def, const char *model);
+virSecurityDeviceLabelDefPtr
+virDomainShmemDefGetSecurityLabelDef(virDomainShmemDefPtr def, const char *model);
+
typedef const char* (*virEventActionToStringFunc)(int type);
typedef int (*virEventActionFromStringFunc)(const char *type);
--
1.8.3.1
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
--
Marc-André Lureau
9:39 p.m.
New subject: [libvirt] [PATCH 1/4] conf: introduce seclabels in shmem device element
Hi Marc-André
On 07/27/2015 11:42 PM, Marc-André Lureau wrote:
Hi
On Thu, Jul 23, 2015 at 12:13 PM, Luyao Huang <lhuang(a)redhat.com> wrote:
> Introduce a new element in shmem device element, this
> could help users to change the shm label to a specified
> label.
>
> Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
> ---
> docs/formatdomain.html.in | 7 ++++++
> docs/schemas/domaincommon.rng | 3 +++
> src/conf/domain_conf.c | 55 ++++++++++++++++++++++++++++++++++---------
> src/conf/domain_conf.h | 5 ++++
> 4 files changed, 59 insertions(+), 11 deletions(-)
>
It would be better with a small test, checking parsing and format.
Oh, right, i forgot that, thanks for pointing out that, i will add them
in next version.
> diff --git a/docs/formatdomain.html.in
b/docs/formatdomain.html.in
> index d0c1741..e02c67c 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -6098,6 +6098,13 @@ qemu-kvm -net nic,model=? /dev/null
> vectors. The <code>ioeventd</code> attribute enables/disables
(values
> "on"/"off", respectively) ioeventfd.
> </dd>
> + <dt><code>seclabel</code></dt>
> + <dd>
> + The optional <code>seclabel</code> to override the way that
labelling
The "element may contain an" optional <code>...
Okay
> + is done on the shm object path or shm server path. If
this
> + element is not present, the <a href="#seclabel">security label
is inherited
> + from the per-domain setting</a>.
> + </dd>
> </dl>
>
> <h4><a name="elementsMemory">Memory
devices</a></h4>
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index 1120003..f58e8de 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -3323,6 +3323,9 @@
> </optional>
> </element>
> </optional>
> + <zeroOrMore>
> + <ref name='devSeclabel'/>
> + </zeroOrMore>
> <optional>
> <ref name="address"/>
> </optional>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 73ac537..cb3d72a 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -11261,6 +11261,8 @@ virDomainNVRAMDefParseXML(xmlNodePtr node,
> static virDomainShmemDefPtr
> virDomainShmemDefParseXML(xmlNodePtr node,
> xmlXPathContextPtr ctxt,
> + virSecurityLabelDefPtr* vmSeclabels,
> + int nvmSeclabels,
> unsigned int flags)
> {
> char *tmp = NULL;
> @@ -11332,6 +11334,10 @@ virDomainShmemDefParseXML(xmlNodePtr node,
> if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0)
> goto cleanup;
>
> + if (virSecurityDeviceLabelDefParseXML(&def->seclabels,
&def->nseclabels,
> + vmSeclabels, nvmSeclabels,
> + ctxt, flags) < 0)
> + goto cleanup;
>
> ret = def;
> def = NULL;
> @@ -12457,7 +12463,11 @@ virDomainDeviceDefParse(const char *xmlStr,
> goto error;
> break;
> case VIR_DOMAIN_DEVICE_SHMEM:
> - if (!(dev->data.shmem = virDomainShmemDefParseXML(node, ctxt, flags)))
> + if (!(dev->data.shmem = virDomainShmemDefParseXML(node,
> + ctxt,
> + def->seclabels,
> + def->nseclabels,
> + flags)))
> goto error;
> break;
> case VIR_DOMAIN_DEVICE_TPM:
> @@ -16136,7 +16146,8 @@ virDomainDefParseXML(xmlDocPtr xml,
> for (i = 0; i < n; i++) {
> virDomainShmemDefPtr shmem;
> ctxt->node = nodes[i];
> - shmem = virDomainShmemDefParseXML(nodes[i], ctxt, flags);
> + shmem = virDomainShmemDefParseXML(nodes[i], ctxt, def->seclabels,
> + def->nseclabels, flags);
> if (!shmem)
> goto error;
>
> @@ -20308,6 +20319,8 @@ virDomainShmemDefFormat(virBufferPtr buf,
> virDomainShmemDefPtr def,
> unsigned int flags)
> {
> + size_t n;
> +
> virBufferEscapeString(buf, "<shmem name='%s'",
def->name);
>
> if (!def->size &&
> @@ -20341,6 +20354,9 @@ virDomainShmemDefFormat(virBufferPtr buf,
> virBufferAddLit(buf, "/>\n");
> }
>
> + for (n = 0; n < def->nseclabels; n++)
> + virSecurityDeviceLabelDefFormat(buf, def->seclabels[n], flags);
> +
> if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
> return -1;
>
> @@ -23851,11 +23867,25 @@ virDomainObjListExport(virDomainObjListPtr domlist,
> }
>
>
> +static virSecurityDeviceLabelDefPtr
> +virDomainGetDeviceSecurityLabelDef(virSecurityDeviceLabelDefPtr *seclabels,
> + size_t nseclabels,
> + const char *model)
> +{
> + size_t i;
> +
> + for (i = 0; i < nseclabels; i++) {
> + if (STREQ_NULLABLE(seclabels[i]->model, model))
> + return seclabels[i];
> + }
> + return NULL;
> +}
> +
> +
> virSecurityLabelDefPtr
> virDomainDefGetSecurityLabelDef(virDomainDefPtr def, const char *model)
> {
> size_t i;
> - virSecurityLabelDefPtr seclabel = NULL;
>
> if (def == NULL || model == NULL)
> return NULL;
> @@ -23866,24 +23896,27 @@ virDomainDefGetSecurityLabelDef(virDomainDefPtr def, const
char *model)
> if (STREQ(def->seclabels[i]->model, model))
> return def->seclabels[i];
> }
> -
> - return seclabel;
> + return NULL;
This looks like a seperate cleanup.
Yes, i will split this in another patch.
Thanks a lot for your review.
Luyao
> }
>
>
> virSecurityDeviceLabelDefPtr
> virDomainChrDefGetSecurityLabelDef(virDomainChrDefPtr def, const char *model)
> {
> - size_t i;
> + if (def == NULL)
> + return NULL;
> +
> + return virDomainGetDeviceSecurityLabelDef(def->seclabels, def->nseclabels,
model);
> +}
>
> +
> +virSecurityDeviceLabelDefPtr
> +virDomainShmemDefGetSecurityLabelDef(virDomainShmemDefPtr def, const char *model)
> +{
> if (def == NULL)
> return NULL;
>
> - for (i = 0; i < def->nseclabels; i++) {
> - if (STREQ_NULLABLE(def->seclabels[i]->model, model))
> - return def->seclabels[i];
> - }
> - return NULL;
> + return virDomainGetDeviceSecurityLabelDef(def->seclabels, def->nseclabels,
model);
> }
>
>
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 0fe6b1a..1a0475e 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -1608,6 +1608,8 @@ struct _virDomainShmemDef {
> unsigned vectors;
> virTristateSwitch ioeventfd;
> } msi;
> + size_t nseclabels;
> + virSecurityDeviceLabelDefPtr *seclabels;
> virDomainDeviceInfo info;
> };
>
> @@ -2943,6 +2945,9 @@ virDomainDefGetSecurityLabelDef(virDomainDefPtr def, const char
*model);
> virSecurityDeviceLabelDefPtr
> virDomainChrDefGetSecurityLabelDef(virDomainChrDefPtr def, const char *model);
>
> +virSecurityDeviceLabelDefPtr
> +virDomainShmemDefGetSecurityLabelDef(virDomainShmemDefPtr def, const char *model);
> +
> typedef const char* (*virEventActionToStringFunc)(int type);
> typedef int (*virEventActionFromStringFunc)(const char *type);
>
> --
> 1.8.3.1
>
> --
> libvir-list mailing list
> libvir-list(a)redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
4:48 a.m.
New subject: [libvirt] [PATCH 1/4] conf: introduce seclabels in shmem device element
On Thu, Jul 23, 2015 at 06:13:46PM +0800, Luyao Huang wrote:
Introduce a new element in shmem device element, this
could help users to change the shm label to a specified
label.
Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
---
docs/formatdomain.html.in | 7 ++++++
docs/schemas/domaincommon.rng | 3 +++
src/conf/domain_conf.c | 55 ++++++++++++++++++++++++++++++++++---------
src/conf/domain_conf.h | 5 ++++
4 files changed, 59 insertions(+), 11 deletions(-)
As already mentioned, this must include additions to the qemu tests
suite for XML to XML conversion.
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 :|
9:39 p.m.
New subject: [libvirt] [PATCH 1/4] conf: introduce seclabels in shmem device element
On 07/30/2015 05:48 PM, Daniel P. Berrange wrote:
On Thu, Jul 23, 2015 at 06:13:46PM +0800, Luyao Huang wrote:
> Introduce a new element in shmem device element, this
> could help users to change the shm label to a specified
> label.
>
> Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
> ---
> docs/formatdomain.html.in | 7 ++++++
> docs/schemas/domaincommon.rng | 3 +++
> src/conf/domain_conf.c | 55 ++++++++++++++++++++++++++++++++++---------
> src/conf/domain_conf.h | 5 ++++
> 4 files changed, 59 insertions(+), 11 deletions(-)
As already mentioned, this must include additions to the qemu tests
suite for XML to XML conversion.
I must forgot this during wrote this patch, thanks for pointing out
that, i will add a tests for the new XML element in next version.
Thanks a lot for your review.
Regards,
Daniel
Luyao
5:28 a.m.
New subject: [libvirt] [PATCH 1/4] conf: introduce seclabels in shmem device element
On Thu, Jul 23, 2015 at 06:13:46PM +0800, Luyao Huang wrote:
Introduce a new element in shmem device element, this
could help users to change the shm label to a specified
label.
Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
---
docs/formatdomain.html.in | 7 ++++++
docs/schemas/domaincommon.rng | 3 +++
src/conf/domain_conf.c | 55 ++++++++++++++++++++++++++++++++++---------
src/conf/domain_conf.h | 5 ++++
4 files changed, 59 insertions(+), 11 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index d0c1741..e02c67c 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -6098,6 +6098,13 @@ qemu-kvm -net nic,model=? /dev/null
vectors. The <code>ioeventd</code> attribute enables/disables (values
"on"/"off", respectively) ioeventfd.
</dd>
+ <dt><code>seclabel</code></dt>
+ <dd>
+ The optional <code>seclabel</code> to override the way that
labelling
+ is done on the shm object path or shm server path. If this
+ element is not present, the <a href="#seclabel">security label is
inherited
+ from the per-domain setting</a>.
+ </dd>
</dl>
<h4><a name="elementsMemory">Memory
devices</a></h4>
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 1120003..f58e8de 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3323,6 +3323,9 @@
</optional>
</element>
</optional>
+ <zeroOrMore>
+ <ref name='devSeclabel'/>
+ </zeroOrMore>
<optional>
<ref name="address"/>
</optional>
So in the <disk> XML we have an explicit element to indicate whether the
device is intended to be shared across multiple guests. <shareable/>
I think we need to have the same flag added to the shm device too, so
that we sanity check whether a particular shm was intended to be shared
or whether it is a mistake when multiple guests use it. This will also
allow us to integrate with the virtlockd to acquire exclusive locks
against the shm device to actively prevent admin mistakes starting
2 guests with the same shm. It will also let us automatically choose
the right default SELinux label ie svirt_image_t:s0:c214,c3242 for
exclusive access vs svirt_image_t:s0 for shared access
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 :|
2:47 a.m.
New subject: [libvirt] [PATCH 1/4] conf: introduce seclabels in shmem device element
On 07/30/2015 06:28 PM, Daniel P. Berrange wrote:
On Thu, Jul 23, 2015 at 06:13:46PM +0800, Luyao Huang wrote:
> Introduce a new element in shmem device element, this
> could help users to change the shm label to a specified
> label.
>
> Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
> ---
> docs/formatdomain.html.in | 7 ++++++
> docs/schemas/domaincommon.rng | 3 +++
> src/conf/domain_conf.c | 55 ++++++++++++++++++++++++++++++++++---------
> src/conf/domain_conf.h | 5 ++++
> 4 files changed, 59 insertions(+), 11 deletions(-)
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index d0c1741..e02c67c 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -6098,6 +6098,13 @@ qemu-kvm -net nic,model=? /dev/null
> vectors. The <code>ioeventd</code> attribute enables/disables
(values
> "on"/"off", respectively) ioeventfd.
> </dd>
> + <dt><code>seclabel</code></dt>
> + <dd>
> + The optional <code>seclabel</code> to override the way that
labelling
> + is done on the shm object path or shm server path. If this
> + element is not present, the <a href="#seclabel">security label
is inherited
> + from the per-domain setting</a>.
> + </dd>
> </dl>
>
> <h4><a name="elementsMemory">Memory
devices</a></h4>
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index 1120003..f58e8de 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -3323,6 +3323,9 @@
> </optional>
> </element>
> </optional>
> + <zeroOrMore>
> + <ref name='devSeclabel'/>
> + </zeroOrMore>
> <optional>
> <ref name="address"/>
> </optional>
So in the <disk> XML we have an explicit element to indicate whether the
device is intended to be shared across multiple guests. <shareable/>
I think we need to have the same flag added to the shm device too, so
that we sanity check whether a particular shm was intended to be shared
or whether it is a mistake when multiple guests use it. This will also
allow us to integrate with the virtlockd to acquire exclusive locks
against the shm device to actively prevent admin mistakes starting
2 guests with the same shm. It will also let us automatically choose
the right default SELinux label ie svirt_image_t:s0:c214,c3242 for
exclusive access vs svirt_image_t:s0 for shared access
Good idea! i will introduce this new element in next version.
Thanks a lot for your advise.
Regards,
Daniel
Luyao
5:13 a.m.
New subject: [libvirt] [PATCH 2/4] security: add security part for shmem device
A new api to help set/restore the shmem deivce dac/selinux label.
Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
---
src/libvirt_private.syms | 2 ++
src/security/security_dac.c | 67 +++++++++++++++++++++++++++++++++++++++
src/security/security_driver.h | 11 +++++++
src/security/security_manager.c | 38 ++++++++++++++++++++++
src/security/security_manager.h | 8 +++++
src/security/security_selinux.c | 70 +++++++++++++++++++++++++++++++++++++++++
src/security/security_stack.c | 41 ++++++++++++++++++++++++
7 files changed, 237 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 588b1c4..af73177 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1038,6 +1038,7 @@ virSecurityManagerRestoreDiskLabel;
virSecurityManagerRestoreHostdevLabel;
virSecurityManagerRestoreImageLabel;
virSecurityManagerRestoreSavedStateLabel;
+virSecurityManagerRestoreShmemLabel;
virSecurityManagerSetAllLabel;
virSecurityManagerSetChildProcessLabel;
virSecurityManagerSetDaemonSocketLabel;
@@ -1048,6 +1049,7 @@ virSecurityManagerSetImageFDLabel;
virSecurityManagerSetImageLabel;
virSecurityManagerSetProcessLabel;
virSecurityManagerSetSavedStateLabel;
+virSecurityManagerSetShmemLabel;
virSecurityManagerSetSocketLabel;
virSecurityManagerSetTapFDLabel;
virSecurityManagerStackAddNested;
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index deb6980..f954aa5 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -39,6 +39,7 @@
#include "virstoragefile.h"
#include "virstring.h"
#include "virutil.h"
+#include "virshm.h"
#define VIR_FROM_THIS VIR_FROM_SECURITY
@@ -922,6 +923,69 @@ virSecurityDACRestoreSecurityTPMFileLabel(virSecurityManagerPtr mgr,
static int
+virSecurityDACSetShmemLabel(virSecurityManagerPtr mgr,
+ virDomainDefPtr def,
+ virDomainShmemDefPtr shmem,
+ char *path)
+{
+ virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
+ virSecurityLabelDefPtr seclabel;
+ virSecurityDeviceLabelDefPtr shmem_seclabel = NULL;
+ char *tmppath;
+ uid_t user;
+ gid_t group;
+
+ if (shmem->server.enabled)
+ tmppath = shmem->server.chr.data.nix.path;
+ else
+ tmppath = path;
+
+ if (!tmppath)
+ return 0;
+
+ shmem_seclabel = virDomainShmemDefGetSecurityLabelDef(shmem, SECURITY_DAC_NAME);
+
+ if (shmem_seclabel && !shmem_seclabel->relabel)
+ return 0;
+
+ seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME);
+
+ if (shmem_seclabel && shmem_seclabel->label) {
+ if (virParseOwnershipIds(shmem_seclabel->label, &user, &group) <
0)
+ return -1;
+ } else {
+ if (virSecurityDACGetIds(seclabel, priv, &user, &group, NULL, NULL) <
0)
+ return -1;
+ }
+
+ return virSecurityDACSetOwnership(tmppath, user, group);
+}
+
+
+static int
+virSecurityDACRestoreShmemLabel(virSecurityManagerPtr mgr,
+ virDomainDefPtr def,
+ virDomainShmemDefPtr shmem,
+ char *path)
+{
+ virSecurityDeviceLabelDefPtr shmem_seclabel = NULL;
+
+ shmem_seclabel = virDomainShmemDefGetSecurityLabelDef(shmem, SECURITY_DAC_NAME);
+
+ if (shmem_seclabel && !shmem_seclabel->relabel)
+ return 0;
+
+ if (shmem->server.enabled)
+ return virSecurityDACRestoreChardevLabel(mgr, def, NULL,
&shmem->server.chr);
+
+ if (!path)
+ return 0;
+
+ return virSecurityDACRestoreSecurityFileLabel(path);
+}
+
+
+static int
virSecurityDACRestoreSecurityAllLabel(virSecurityManagerPtr mgr,
virDomainDefPtr def,
bool migrated)
@@ -1433,4 +1497,7 @@ virSecurityDriver virSecurityDriverDAC = {
.domainGetSecurityMountOptions = virSecurityDACGetMountOptions,
.getBaseLabel = virSecurityDACGetBaseLabel,
+
+ .domainSetSecurityShmemLabel = virSecurityDACSetShmemLabel,
+ .domainRestoreSecurityShmemLabel = virSecurityDACRestoreShmemLabel,
};
diff --git a/src/security/security_driver.h b/src/security/security_driver.h
index f0dca09..37e4527 100644
--- a/src/security/security_driver.h
+++ b/src/security/security_driver.h
@@ -118,6 +118,14 @@ typedef int (*virSecurityDomainSetImageLabel) (virSecurityManagerPtr
mgr,
typedef int (*virSecurityDomainRestoreImageLabel) (virSecurityManagerPtr mgr,
virDomainDefPtr def,
virStorageSourcePtr src);
+typedef int (*virSecurityDomainSetShmemLabel) (virSecurityManagerPtr mgr,
+ virDomainDefPtr def,
+ virDomainShmemDefPtr shmem,
+ char *path);
+typedef int (*virSecurityDomainRestoreShmemLabel) (virSecurityManagerPtr mgr,
+ virDomainDefPtr def,
+ virDomainShmemDefPtr shmem,
+ char *path);
struct _virSecurityDriver {
@@ -168,6 +176,9 @@ struct _virSecurityDriver {
virSecurityDomainSetHugepages domainSetSecurityHugepages;
virSecurityDriverGetBaseLabel getBaseLabel;
+
+ virSecurityDomainSetShmemLabel domainSetSecurityShmemLabel;
+ virSecurityDomainRestoreShmemLabel domainRestoreSecurityShmemLabel;
};
virSecurityDriverPtr virSecurityDriverLookup(const char *name,
diff --git a/src/security/security_manager.c b/src/security/security_manager.c
index b0cd9e8..72ca7e2 100644
--- a/src/security/security_manager.c
+++ b/src/security/security_manager.c
@@ -991,3 +991,41 @@ virSecurityManagerSetHugepages(virSecurityManagerPtr mgr,
return 0;
}
+
+
+int
+virSecurityManagerRestoreShmemLabel(virSecurityManagerPtr mgr,
+ virDomainDefPtr vm,
+ virDomainShmemDefPtr shmem,
+ char *path)
+{
+ if (mgr->drv->domainRestoreSecurityShmemLabel) {
+ int ret;
+ virObjectLock(mgr);
+ ret = mgr->drv->domainRestoreSecurityShmemLabel(mgr, vm, shmem, path);
+ virObjectUnlock(mgr);
+ return ret;
+ }
+
+ virReportUnsupportedError();
+ return -1;
+}
+
+
+int
+virSecurityManagerSetShmemLabel(virSecurityManagerPtr mgr,
+ virDomainDefPtr vm,
+ virDomainShmemDefPtr shmem,
+ char *path)
+{
+ if (mgr->drv->domainSetSecurityShmemLabel) {
+ int ret;
+ virObjectLock(mgr);
+ ret = mgr->drv->domainSetSecurityShmemLabel(mgr, vm, shmem, path);
+ virObjectUnlock(mgr);
+ return ret;
+ }
+
+ virReportUnsupportedError();
+ return -1;
+}
diff --git a/src/security/security_manager.h b/src/security/security_manager.h
index 13468db..ce37c91 100644
--- a/src/security/security_manager.h
+++ b/src/security/security_manager.h
@@ -149,5 +149,13 @@ int virSecurityManagerSetImageLabel(virSecurityManagerPtr mgr,
int virSecurityManagerRestoreImageLabel(virSecurityManagerPtr mgr,
virDomainDefPtr vm,
virStorageSourcePtr src);
+int virSecurityManagerRestoreShmemLabel(virSecurityManagerPtr mgr,
+ virDomainDefPtr vm,
+ virDomainShmemDefPtr shmem,
+ char *path);
+int virSecurityManagerSetShmemLabel(virSecurityManagerPtr mgr,
+ virDomainDefPtr vm,
+ virDomainShmemDefPtr shmem,
+ char *path);
#endif /* VIR_SECURITY_MANAGER_H__ */
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 6e67a86..cbf89ee 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -46,6 +46,7 @@
#include "virconf.h"
#include "virtpm.h"
#include "virstring.h"
+#include "virshm.h"
#define VIR_FROM_THIS VIR_FROM_SECURITY
@@ -1888,6 +1889,37 @@ virSecuritySELinuxRestoreSecuritySmartcardCallback(virDomainDefPtr
def,
}
+static int
+virSecuritySELinuxRestoreShmemLabel(virSecurityManagerPtr mgr,
+ virDomainDefPtr def,
+ virDomainShmemDefPtr shmem,
+ char *path)
+{
+ char *tmppath = NULL;
+ virSecurityLabelDefPtr seclabel;
+ virSecurityDeviceLabelDefPtr shmem_seclabel = NULL;
+
+ seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME);
+ if (!seclabel || !seclabel->relabel)
+ return 0;
+
+ shmem_seclabel = virDomainShmemDefGetSecurityLabelDef(shmem, SECURITY_SELINUX_NAME);
+
+ if (shmem_seclabel && !shmem_seclabel->relabel)
+ return 0;
+
+ if (shmem->server.enabled)
+ tmppath = shmem->server.chr.data.nix.path;
+ else
+ tmppath = path;
+
+ if (!tmppath)
+ return 0;
+
+ return virSecuritySELinuxRestoreSecurityFileLabel(mgr, tmppath);
+}
+
+
static const char *
virSecuritySELinuxGetBaseLabel(virSecurityManagerPtr mgr, int virtType)
{
@@ -2284,6 +2316,41 @@ virSecuritySELinuxSetSecuritySmartcardCallback(virDomainDefPtr
def,
static int
+virSecuritySELinuxSetShmemLabel(virSecurityManagerPtr mgr,
+ virDomainDefPtr def,
+ virDomainShmemDefPtr shmem,
+ char *path)
+{
+ virSecuritySELinuxDataPtr data = virSecurityManagerGetPrivateData(mgr);
+ char *tmppath = NULL;
+ virSecurityLabelDefPtr seclabel;
+ virSecurityDeviceLabelDefPtr shmem_seclabel = NULL;
+
+ seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME);
+ if (!seclabel || !seclabel->relabel)
+ return 0;
+
+ shmem_seclabel = virDomainShmemDefGetSecurityLabelDef(shmem, SECURITY_SELINUX_NAME);
+
+ if (shmem_seclabel && !shmem_seclabel->relabel)
+ return 0;
+
+ if (shmem->server.enabled)
+ tmppath = shmem->server.chr.data.nix.path;
+ else
+ tmppath = path;
+
+ if (!tmppath)
+ return 0;
+
+ if (shmem_seclabel && shmem_seclabel->label)
+ return virSecuritySELinuxSetFilecon(tmppath, shmem_seclabel->label);
+ else
+ return virSecuritySELinuxSetFilecon(tmppath, data->file_context);
+}
+
+
+static int
virSecuritySELinuxSetSecurityAllLabel(virSecurityManagerPtr mgr,
virDomainDefPtr def,
const char *stdin_path)
@@ -2549,4 +2616,7 @@ virSecurityDriver virSecurityDriverSELinux = {
.domainGetSecurityMountOptions = virSecuritySELinuxGetSecurityMountOptions,
.getBaseLabel = virSecuritySELinuxGetBaseLabel,
+
+ .domainSetSecurityShmemLabel = virSecuritySELinuxSetShmemLabel,
+ .domainRestoreSecurityShmemLabel = virSecuritySELinuxRestoreShmemLabel,
};
diff --git a/src/security/security_stack.c b/src/security/security_stack.c
index 1ded57b..22c1b56 100644
--- a/src/security/security_stack.c
+++ b/src/security/security_stack.c
@@ -599,6 +599,44 @@ virSecurityStackRestoreSecurityImageLabel(virSecurityManagerPtr mgr,
return rc;
}
+static int
+virSecurityStackSetShmemLabel(virSecurityManagerPtr mgr,
+ virDomainDefPtr vm,
+ virDomainShmemDefPtr shmem,
+ char *path)
+{
+ virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr);
+ virSecurityStackItemPtr item = priv->itemsHead;
+ int rc = 0;
+
+ for (; item; item = item->next) {
+ if (virSecurityManagerSetShmemLabel(item->securityManager,
+ vm, shmem, path) < 0)
+ rc = -1;
+ }
+
+ return rc;
+}
+
+static int
+virSecurityStackRestoreShmemLabel(virSecurityManagerPtr mgr,
+ virDomainDefPtr vm,
+ virDomainShmemDefPtr shmem,
+ char *path)
+{
+ virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr);
+ virSecurityStackItemPtr item = priv->itemsHead;
+ int rc = 0;
+
+ for (; item; item = item->next) {
+ if (virSecurityManagerRestoreShmemLabel(item->securityManager,
+ vm, shmem, path) < 0)
+ rc = -1;
+ }
+
+ return rc;
+}
+
virSecurityDriver virSecurityDriverStack = {
.privateDataLen = sizeof(virSecurityStackData),
.name = "stack",
@@ -648,4 +686,7 @@ virSecurityDriver virSecurityDriverStack = {
.domainSetSecurityHugepages = virSecurityStackSetHugepages,
.getBaseLabel = virSecurityStackGetBaseLabel,
+
+ .domainSetSecurityShmemLabel = virSecurityStackSetShmemLabel,
+ .domainRestoreSecurityShmemLabel = virSecurityStackRestoreShmemLabel,
};
--
1.8.3.1
10:39 a.m.
New subject: [libvirt] [PATCH 2/4] security: add security part for shmem device
Hi
On Thu, Jul 23, 2015 at 12:13 PM, Luyao Huang <lhuang(a)redhat.com> wrote:
A new api to help set/restore the shmem deivce dac/selinux label.
typo: deivce / device.
Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
---
src/libvirt_private.syms | 2 ++
src/security/security_dac.c | 67 +++++++++++++++++++++++++++++++++++++++
src/security/security_driver.h | 11 +++++++
src/security/security_manager.c | 38 ++++++++++++++++++++++
src/security/security_manager.h | 8 +++++
src/security/security_selinux.c | 70 +++++++++++++++++++++++++++++++++++++++++
src/security/security_stack.c | 41 ++++++++++++++++++++++++
7 files changed, 237 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 588b1c4..af73177 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1038,6 +1038,7 @@ virSecurityManagerRestoreDiskLabel;
virSecurityManagerRestoreHostdevLabel;
virSecurityManagerRestoreImageLabel;
virSecurityManagerRestoreSavedStateLabel;
+virSecurityManagerRestoreShmemLabel;
virSecurityManagerSetAllLabel;
virSecurityManagerSetChildProcessLabel;
virSecurityManagerSetDaemonSocketLabel;
@@ -1048,6 +1049,7 @@ virSecurityManagerSetImageFDLabel;
virSecurityManagerSetImageLabel;
virSecurityManagerSetProcessLabel;
virSecurityManagerSetSavedStateLabel;
+virSecurityManagerSetShmemLabel;
virSecurityManagerSetSocketLabel;
virSecurityManagerSetTapFDLabel;
virSecurityManagerStackAddNested;
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index deb6980..f954aa5 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -39,6 +39,7 @@
#include "virstoragefile.h"
#include "virstring.h"
#include "virutil.h"
+#include "virshm.h"
This header doesn't exist (yet)
#define VIR_FROM_THIS VIR_FROM_SECURITY
@@ -922,6 +923,69 @@ virSecurityDACRestoreSecurityTPMFileLabel(virSecurityManagerPtr
mgr,
static int
+virSecurityDACSetShmemLabel(virSecurityManagerPtr mgr,
+ virDomainDefPtr def,
+ virDomainShmemDefPtr shmem,
+ char *path)
+{
+ virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
+ virSecurityLabelDefPtr seclabel;
+ virSecurityDeviceLabelDefPtr shmem_seclabel = NULL;
+ char *tmppath;
could make it const
+ uid_t user;
+ gid_t group;
+
+ if (shmem->server.enabled)
+ tmppath = shmem->server.chr.data.nix.path;
+ else
+ tmppath = path;
+
+ if (!tmppath)
+ return 0;
+
+ shmem_seclabel = virDomainShmemDefGetSecurityLabelDef(shmem, SECURITY_DAC_NAME);
+
+ if (shmem_seclabel && !shmem_seclabel->relabel)
+ return 0;
+
+ seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME);
+
The function is similar to virSecurityDACSetSecurityImageLabel and yet
subtly different: there is a early dynamicOwnership condition that
seems to be general, the domain seclabel->relabel is checked first. It
would be nice to align the behaviour.
+ if (shmem_seclabel && shmem_seclabel->label) {
+ if (virParseOwnershipIds(shmem_seclabel->label, &user, &group) <
0)
+ return -1;
+ } else {
+ if (virSecurityDACGetIds(seclabel, priv, &user, &group, NULL, NULL) <
0)
+ return -1;
+ }
+
+ return virSecurityDACSetOwnership(tmppath, user, group);
+}
+
+
+static int
+virSecurityDACRestoreShmemLabel(virSecurityManagerPtr mgr,
+ virDomainDefPtr def,
+ virDomainShmemDefPtr shmem,
+ char *path)
+{
+ virSecurityDeviceLabelDefPtr shmem_seclabel = NULL;
+
+ shmem_seclabel = virDomainShmemDefGetSecurityLabelDef(shmem, SECURITY_DAC_NAME);
+
+ if (shmem_seclabel && !shmem_seclabel->relabel)
+ return 0;
+
+ if (shmem->server.enabled)
+ return virSecurityDACRestoreChardevLabel(mgr, def, NULL,
&shmem->server.chr);
+
+ if (!path)
+ return 0;
+
+ return virSecurityDACRestoreSecurityFileLabel(path);
+}
+
+
+static int
virSecurityDACRestoreSecurityAllLabel(virSecurityManagerPtr mgr,
virDomainDefPtr def,
bool migrated)
@@ -1433,4 +1497,7 @@ virSecurityDriver virSecurityDriverDAC = {
.domainGetSecurityMountOptions = virSecurityDACGetMountOptions,
.getBaseLabel = virSecurityDACGetBaseLabel,
+
+ .domainSetSecurityShmemLabel = virSecurityDACSetShmemLabel,
+ .domainRestoreSecurityShmemLabel = virSecurityDACRestoreShmemLabel,
};
diff --git a/src/security/security_driver.h b/src/security/security_driver.h
index f0dca09..37e4527 100644
--- a/src/security/security_driver.h
+++ b/src/security/security_driver.h
@@ -118,6 +118,14 @@ typedef int (*virSecurityDomainSetImageLabel) (virSecurityManagerPtr
mgr,
typedef int (*virSecurityDomainRestoreImageLabel) (virSecurityManagerPtr mgr,
virDomainDefPtr def,
virStorageSourcePtr src);
+typedef int (*virSecurityDomainSetShmemLabel) (virSecurityManagerPtr mgr,
+ virDomainDefPtr def,
+ virDomainShmemDefPtr shmem,
+ char *path);
+typedef int (*virSecurityDomainRestoreShmemLabel) (virSecurityManagerPtr mgr,
+ virDomainDefPtr def,
+ virDomainShmemDefPtr shmem,
+ char *path);
struct _virSecurityDriver {
@@ -168,6 +176,9 @@ struct _virSecurityDriver {
virSecurityDomainSetHugepages domainSetSecurityHugepages;
virSecurityDriverGetBaseLabel getBaseLabel;
+
+ virSecurityDomainSetShmemLabel domainSetSecurityShmemLabel;
+ virSecurityDomainRestoreShmemLabel domainRestoreSecurityShmemLabel;
};
virSecurityDriverPtr virSecurityDriverLookup(const char *name,
diff --git a/src/security/security_manager.c b/src/security/security_manager.c
index b0cd9e8..72ca7e2 100644
--- a/src/security/security_manager.c
+++ b/src/security/security_manager.c
@@ -991,3 +991,41 @@ virSecurityManagerSetHugepages(virSecurityManagerPtr mgr,
return 0;
}
+
+
+int
+virSecurityManagerRestoreShmemLabel(virSecurityManagerPtr mgr,
+ virDomainDefPtr vm,
+ virDomainShmemDefPtr shmem,
+ char *path)
+{
+ if (mgr->drv->domainRestoreSecurityShmemLabel) {
+ int ret;
+ virObjectLock(mgr);
+ ret = mgr->drv->domainRestoreSecurityShmemLabel(mgr, vm, shmem, path);
+ virObjectUnlock(mgr);
+ return ret;
+ }
+
+ virReportUnsupportedError();
+ return -1;
+}
+
+
+int
+virSecurityManagerSetShmemLabel(virSecurityManagerPtr mgr,
+ virDomainDefPtr vm,
+ virDomainShmemDefPtr shmem,
+ char *path)
+{
+ if (mgr->drv->domainSetSecurityShmemLabel) {
+ int ret;
+ virObjectLock(mgr);
+ ret = mgr->drv->domainSetSecurityShmemLabel(mgr, vm, shmem, path);
+ virObjectUnlock(mgr);
+ return ret;
+ }
+
+ virReportUnsupportedError();
+ return -1;
+}
diff --git a/src/security/security_manager.h b/src/security/security_manager.h
index 13468db..ce37c91 100644
--- a/src/security/security_manager.h
+++ b/src/security/security_manager.h
@@ -149,5 +149,13 @@ int virSecurityManagerSetImageLabel(virSecurityManagerPtr mgr,
int virSecurityManagerRestoreImageLabel(virSecurityManagerPtr mgr,
virDomainDefPtr vm,
virStorageSourcePtr src);
+int virSecurityManagerRestoreShmemLabel(virSecurityManagerPtr mgr,
+ virDomainDefPtr vm,
+ virDomainShmemDefPtr shmem,
+ char *path);
+int virSecurityManagerSetShmemLabel(virSecurityManagerPtr mgr,
+ virDomainDefPtr vm,
+ virDomainShmemDefPtr shmem,
+ char *path);
const path
#endif /* VIR_SECURITY_MANAGER_H__ */
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 6e67a86..cbf89ee 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -46,6 +46,7 @@
#include "virconf.h"
#include "virtpm.h"
#include "virstring.h"
+#include "virshm.h"
remove that too
#define VIR_FROM_THIS VIR_FROM_SECURITY
@@ -1888,6 +1889,37 @@ virSecuritySELinuxRestoreSecuritySmartcardCallback(virDomainDefPtr
def,
}
+static int
+virSecuritySELinuxRestoreShmemLabel(virSecurityManagerPtr mgr,
+ virDomainDefPtr def,
+ virDomainShmemDefPtr shmem,
+ char *path)
const path
+{
+ char *tmppath = NULL;
make it const too
+ virSecurityLabelDefPtr seclabel;
+ virSecurityDeviceLabelDefPtr shmem_seclabel = NULL;
+
+ seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME);
+ if (!seclabel || !seclabel->relabel)
+ return 0;
+
+ shmem_seclabel = virDomainShmemDefGetSecurityLabelDef(shmem,
SECURITY_SELINUX_NAME);
+
+ if (shmem_seclabel && !shmem_seclabel->relabel)
+ return 0;
+
+ if (shmem->server.enabled)
+ tmppath = shmem->server.chr.data.nix.path;
+ else
+ tmppath = path;
+
+ if (!tmppath)
+ return 0;
+
+ return virSecuritySELinuxRestoreSecurityFileLabel(mgr, tmppath);
+}
+
+
static const char *
virSecuritySELinuxGetBaseLabel(virSecurityManagerPtr mgr, int virtType)
{
@@ -2284,6 +2316,41 @@ virSecuritySELinuxSetSecuritySmartcardCallback(virDomainDefPtr
def,
static int
+virSecuritySELinuxSetShmemLabel(virSecurityManagerPtr mgr,
+ virDomainDefPtr def,
+ virDomainShmemDefPtr shmem,
+ char *path)
+{
+ virSecuritySELinuxDataPtr data = virSecurityManagerGetPrivateData(mgr);
+ char *tmppath = NULL;
+ virSecurityLabelDefPtr seclabel;
+ virSecurityDeviceLabelDefPtr shmem_seclabel = NULL;
+
+ seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME);
+ if (!seclabel || !seclabel->relabel)
+ return 0;
+
+ shmem_seclabel = virDomainShmemDefGetSecurityLabelDef(shmem,
SECURITY_SELINUX_NAME);
+
+ if (shmem_seclabel && !shmem_seclabel->relabel)
+ return 0;
+
+ if (shmem->server.enabled)
+ tmppath = shmem->server.chr.data.nix.path;
+ else
+ tmppath = path;
I am not sure it's a good idea to either set the server socket policy
or the shm. Why not set both?
+ if (!tmppath)
+ return 0;
+
+ if (shmem_seclabel && shmem_seclabel->label)
+ return virSecuritySELinuxSetFilecon(tmppath, shmem_seclabel->label);
+ else
+ return virSecuritySELinuxSetFilecon(tmppath, data->file_context);
+}
+
+
+static int
virSecuritySELinuxSetSecurityAllLabel(virSecurityManagerPtr mgr,
virDomainDefPtr def,
const char *stdin_path)
@@ -2549,4 +2616,7 @@ virSecurityDriver virSecurityDriverSELinux = {
.domainGetSecurityMountOptions = virSecuritySELinuxGetSecurityMountOptions,
.getBaseLabel = virSecuritySELinuxGetBaseLabel,
+
+ .domainSetSecurityShmemLabel = virSecuritySELinuxSetShmemLabel,
+ .domainRestoreSecurityShmemLabel = virSecuritySELinuxRestoreShmemLabel,
};
diff --git a/src/security/security_stack.c b/src/security/security_stack.c
index 1ded57b..22c1b56 100644
--- a/src/security/security_stack.c
+++ b/src/security/security_stack.c
@@ -599,6 +599,44 @@ virSecurityStackRestoreSecurityImageLabel(virSecurityManagerPtr
mgr,
return rc;
}
+static int
+virSecurityStackSetShmemLabel(virSecurityManagerPtr mgr,
+ virDomainDefPtr vm,
+ virDomainShmemDefPtr shmem,
+ char *path)
+{
+ virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr);
+ virSecurityStackItemPtr item = priv->itemsHead;
+ int rc = 0;
+
+ for (; item; item = item->next) {
+ if (virSecurityManagerSetShmemLabel(item->securityManager,
+ vm, shmem, path) < 0)
+ rc = -1;
+ }
+
+ return rc;
+}
+
+static int
+virSecurityStackRestoreShmemLabel(virSecurityManagerPtr mgr,
+ virDomainDefPtr vm,
+ virDomainShmemDefPtr shmem,
+ char *path)
+{
+ virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr);
+ virSecurityStackItemPtr item = priv->itemsHead;
+ int rc = 0;
+
+ for (; item; item = item->next) {
+ if (virSecurityManagerRestoreShmemLabel(item->securityManager,
+ vm, shmem, path) < 0)
+ rc = -1;
+ }
+
+ return rc;
+}
+
virSecurityDriver virSecurityDriverStack = {
.privateDataLen = sizeof(virSecurityStackData),
.name = "stack",
@@ -648,4 +686,7 @@ virSecurityDriver virSecurityDriverStack = {
.domainSetSecurityHugepages = virSecurityStackSetHugepages,
.getBaseLabel = virSecurityStackGetBaseLabel,
+
+ .domainSetSecurityShmemLabel = virSecurityStackSetShmemLabel,
+ .domainRestoreSecurityShmemLabel = virSecurityStackRestoreShmemLabel,
};
--
1.8.3.1
Shouldn't it be implemented for the nop virSecurityDriver too? (note:
I don't know what it is for)
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
--
Marc-André Lureau
7346 2483 9404 4E20 ABFF 7D48 D864 9487 F43F 0992
6:27 a.m.
New subject: [libvirt] [PATCH 2/4] security: add security part for shmem device
Hi Marc-André
On 07/27/2015 11:39 PM, Marc-André Lureau wrote:
Hi
On Thu, Jul 23, 2015 at 12:13 PM, Luyao Huang<lhuang(a)redhat.com> wrote:
> A new api to help set/restore the shmem deivce dac/selinux label.
typo: deivce / device.
thanks, i will fix this in next version
> Signed-off-by: Luyao Huang<lhuang(a)redhat.com>
> ---
> src/libvirt_private.syms | 2 ++
> src/security/security_dac.c | 67 +++++++++++++++++++++++++++++++++++++++
> src/security/security_driver.h | 11 +++++++
> src/security/security_manager.c | 38 ++++++++++++++++++++++
> src/security/security_manager.h | 8 +++++
> src/security/security_selinux.c | 70 +++++++++++++++++++++++++++++++++++++++++
> src/security/security_stack.c | 41 ++++++++++++++++++++++++
> 7 files changed, 237 insertions(+)
>
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 588b1c4..af73177 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1038,6 +1038,7 @@ virSecurityManagerRestoreDiskLabel;
> virSecurityManagerRestoreHostdevLabel;
> virSecurityManagerRestoreImageLabel;
> virSecurityManagerRestoreSavedStateLabel;
> +virSecurityManagerRestoreShmemLabel;
> virSecurityManagerSetAllLabel;
> virSecurityManagerSetChildProcessLabel;
> virSecurityManagerSetDaemonSocketLabel;
> @@ -1048,6 +1049,7 @@ virSecurityManagerSetImageFDLabel;
> virSecurityManagerSetImageLabel;
> virSecurityManagerSetProcessLabel;
> virSecurityManagerSetSavedStateLabel;
> +virSecurityManagerSetShmemLabel;
> virSecurityManagerSetSocketLabel;
> virSecurityManagerSetTapFDLabel;
> virSecurityManagerStackAddNested;
> diff --git a/src/security/security_dac.c b/src/security/security_dac.c
> index deb6980..f954aa5 100644
> --- a/src/security/security_dac.c
> +++ b/src/security/security_dac.c
> @@ -39,6 +39,7 @@
> #include "virstoragefile.h"
> #include "virstring.h"
> #include "virutil.h"
> +#include "virshm.h"
This header doesn't exist (yet)
I forgot remove this, thanks for pointing out. (and i won't remove this,
since i need a function in virshm.h, so i will move it this patch after
patch 3/4 in next version)
> #define VIR_FROM_THIS VIR_FROM_SECURITY
>
> @@ -922,6 +923,69 @@ virSecurityDACRestoreSecurityTPMFileLabel(virSecurityManagerPtr
mgr,
>
>
> static int
> +virSecurityDACSetShmemLabel(virSecurityManagerPtr mgr,
> + virDomainDefPtr def,
> + virDomainShmemDefPtr shmem,
> + char *path)
> +{
> + virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
> + virSecurityLabelDefPtr seclabel;
> + virSecurityDeviceLabelDefPtr shmem_seclabel = NULL;
> + char *tmppath;
could make it const
Okay
> + uid_t user;
> + gid_t group;
> +
> + if (shmem->server.enabled)
> + tmppath = shmem->server.chr.data.nix.path;
> + else
> + tmppath = path;
> +
> + if (!tmppath)
> + return 0;
> +
> + shmem_seclabel = virDomainShmemDefGetSecurityLabelDef(shmem,
SECURITY_DAC_NAME);
> +
> + if (shmem_seclabel && !shmem_seclabel->relabel)
> + return 0;
> +
> + seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME);
> +
The function is similar to virSecurityDACSetSecurityImageLabel and yet
subtly different: there is a early dynamicOwnership condition that
seems to be general, the domain seclabel->relabel is checked first. It
would be nice to align the behaviour.
Indeed, i will move the shmem_seclabel and seclabel check more early.
> + if (shmem_seclabel && shmem_seclabel->label) {
> + if (virParseOwnershipIds(shmem_seclabel->label, &user, &group)
< 0)
> + return -1;
> + } else {
> + if (virSecurityDACGetIds(seclabel, priv, &user, &group, NULL, NULL)
< 0)
> + return -1;
> + }
> +
> + return virSecurityDACSetOwnership(tmppath, user, group);
> +}
> +
> +
> +static int
> +virSecurityDACRestoreShmemLabel(virSecurityManagerPtr mgr,
> + virDomainDefPtr def,
> + virDomainShmemDefPtr shmem,
> + char *path)
> +{
> + virSecurityDeviceLabelDefPtr shmem_seclabel = NULL;
> +
> + shmem_seclabel = virDomainShmemDefGetSecurityLabelDef(shmem,
SECURITY_DAC_NAME);
> +
> + if (shmem_seclabel && !shmem_seclabel->relabel)
> + return 0;
> +
> + if (shmem->server.enabled)
> + return virSecurityDACRestoreChardevLabel(mgr, def, NULL,
&shmem->server.chr);
> +
> + if (!path)
> + return 0;
> +
> + return virSecurityDACRestoreSecurityFileLabel(path);
> +}
> +
> +
> +static int
> virSecurityDACRestoreSecurityAllLabel(virSecurityManagerPtr mgr,
> virDomainDefPtr def,
> bool migrated)
> @@ -1433,4 +1497,7 @@ virSecurityDriver virSecurityDriverDAC = {
> .domainGetSecurityMountOptions = virSecurityDACGetMountOptions,
>
> .getBaseLabel = virSecurityDACGetBaseLabel,
> +
> + .domainSetSecurityShmemLabel = virSecurityDACSetShmemLabel,
> + .domainRestoreSecurityShmemLabel = virSecurityDACRestoreShmemLabel,
> };
> diff --git a/src/security/security_driver.h b/src/security/security_driver.h
> index f0dca09..37e4527 100644
> --- a/src/security/security_driver.h
> +++ b/src/security/security_driver.h
> @@ -118,6 +118,14 @@ typedef int (*virSecurityDomainSetImageLabel)
(virSecurityManagerPtr mgr,
> typedef int (*virSecurityDomainRestoreImageLabel) (virSecurityManagerPtr mgr,
> virDomainDefPtr def,
> virStorageSourcePtr src);
> +typedef int (*virSecurityDomainSetShmemLabel) (virSecurityManagerPtr mgr,
> + virDomainDefPtr def,
> + virDomainShmemDefPtr shmem,
> + char *path);
> +typedef int (*virSecurityDomainRestoreShmemLabel) (virSecurityManagerPtr mgr,
> + virDomainDefPtr def,
> + virDomainShmemDefPtr shmem,
> + char *path);
>
>
> struct _virSecurityDriver {
> @@ -168,6 +176,9 @@ struct _virSecurityDriver {
> virSecurityDomainSetHugepages domainSetSecurityHugepages;
>
> virSecurityDriverGetBaseLabel getBaseLabel;
> +
> + virSecurityDomainSetShmemLabel domainSetSecurityShmemLabel;
> + virSecurityDomainRestoreShmemLabel domainRestoreSecurityShmemLabel;
> };
>
> virSecurityDriverPtr virSecurityDriverLookup(const char *name,
> diff --git a/src/security/security_manager.c b/src/security/security_manager.c
> index b0cd9e8..72ca7e2 100644
> --- a/src/security/security_manager.c
> +++ b/src/security/security_manager.c
> @@ -991,3 +991,41 @@ virSecurityManagerSetHugepages(virSecurityManagerPtr mgr,
>
> return 0;
> }
> +
> +
> +int
> +virSecurityManagerRestoreShmemLabel(virSecurityManagerPtr mgr,
> + virDomainDefPtr vm,
> + virDomainShmemDefPtr shmem,
> + char *path)
> +{
> + if (mgr->drv->domainRestoreSecurityShmemLabel) {
> + int ret;
> + virObjectLock(mgr);
> + ret = mgr->drv->domainRestoreSecurityShmemLabel(mgr, vm, shmem,
path);
> + virObjectUnlock(mgr);
> + return ret;
> + }
> +
> + virReportUnsupportedError();
> + return -1;
> +}
> +
> +
> +int
> +virSecurityManagerSetShmemLabel(virSecurityManagerPtr mgr,
> + virDomainDefPtr vm,
> + virDomainShmemDefPtr shmem,
> + char *path)
> +{
> + if (mgr->drv->domainSetSecurityShmemLabel) {
> + int ret;
> + virObjectLock(mgr);
> + ret = mgr->drv->domainSetSecurityShmemLabel(mgr, vm, shmem, path);
> + virObjectUnlock(mgr);
> + return ret;
> + }
> +
> + virReportUnsupportedError();
> + return -1;
> +}
> diff --git a/src/security/security_manager.h b/src/security/security_manager.h
> index 13468db..ce37c91 100644
> --- a/src/security/security_manager.h
> +++ b/src/security/security_manager.h
> @@ -149,5 +149,13 @@ int virSecurityManagerSetImageLabel(virSecurityManagerPtr mgr,
> int virSecurityManagerRestoreImageLabel(virSecurityManagerPtr mgr,
> virDomainDefPtr vm,
> virStorageSourcePtr src);
> +int virSecurityManagerRestoreShmemLabel(virSecurityManagerPtr mgr,
> + virDomainDefPtr vm,
> + virDomainShmemDefPtr shmem,
> + char *path);
> +int virSecurityManagerSetShmemLabel(virSecurityManagerPtr mgr,
> + virDomainDefPtr vm,
> + virDomainShmemDefPtr shmem,
> + char *path);
const path
Okay
> #endif /* VIR_SECURITY_MANAGER_H__ */
> diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
> index 6e67a86..cbf89ee 100644
> --- a/src/security/security_selinux.c
> +++ b/src/security/security_selinux.c
> @@ -46,6 +46,7 @@
> #include "virconf.h"
> #include "virtpm.h"
> #include "virstring.h"
> +#include "virshm.h"
remove that too
> #define VIR_FROM_THIS VIR_FROM_SECURITY
>
> @@ -1888,6 +1889,37 @@
virSecuritySELinuxRestoreSecuritySmartcardCallback(virDomainDefPtr def,
> }
>
>
> +static int
> +virSecuritySELinuxRestoreShmemLabel(virSecurityManagerPtr mgr,
> + virDomainDefPtr def,
> + virDomainShmemDefPtr shmem,
> + char *path)
const path
> +{
> + char *tmppath = NULL;
make it const too
> + virSecurityLabelDefPtr seclabel;
> + virSecurityDeviceLabelDefPtr shmem_seclabel = NULL;
> +
> + seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME);
> + if (!seclabel || !seclabel->relabel)
> + return 0;
> +
> + shmem_seclabel = virDomainShmemDefGetSecurityLabelDef(shmem,
SECURITY_SELINUX_NAME);
> +
> + if (shmem_seclabel && !shmem_seclabel->relabel)
> + return 0;
> +
> + if (shmem->server.enabled)
> + tmppath = shmem->server.chr.data.nix.path;
> + else
> + tmppath = path;
> +
> + if (!tmppath)
> + return 0;
> +
> + return virSecuritySELinuxRestoreSecurityFileLabel(mgr, tmppath);
> +}
> +
> +
> static const char *
> virSecuritySELinuxGetBaseLabel(virSecurityManagerPtr mgr, int virtType)
> {
> @@ -2284,6 +2316,41 @@ virSecuritySELinuxSetSecuritySmartcardCallback(virDomainDefPtr
def,
>
>
> static int
> +virSecuritySELinuxSetShmemLabel(virSecurityManagerPtr mgr,
> + virDomainDefPtr def,
> + virDomainShmemDefPtr shmem,
> + char *path)
> +{
> + virSecuritySELinuxDataPtr data = virSecurityManagerGetPrivateData(mgr);
> + char *tmppath = NULL;
> + virSecurityLabelDefPtr seclabel;
> + virSecurityDeviceLabelDefPtr shmem_seclabel = NULL;
> +
> + seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME);
> + if (!seclabel || !seclabel->relabel)
> + return 0;
> +
> + shmem_seclabel = virDomainShmemDefGetSecurityLabelDef(shmem,
SECURITY_SELINUX_NAME);
> +
> + if (shmem_seclabel && !shmem_seclabel->relabel)
> + return 0;
> +
> + if (shmem->server.enabled)
> + tmppath = shmem->server.chr.data.nix.path;
> + else
> + tmppath = path;
I am not sure it's a good idea to either set the server socket policy
or the shm. Why not set both?
Hmm...these $path are the shm path, if the shmem device is server
enabled, the used shm is created by ivshmem-server, which will depends
on ivshmem-server's label. And qemu process will connect to the socket
created by ivshmem-server, so change the label for shm created by
ivshmem-sever is useless (also we don't know the shm name created by
ivshmem-server).
However, never mind, i will do a refactor and remove the variable $path
here to make this function easy to put in
virSecuritySELinuxSetSecurityAllLabel and
virSecuritySELinuxSetSecurityAllLabel.
> + if (!tmppath)
> + return 0;
> +
> + if (shmem_seclabel && shmem_seclabel->label)
> + return virSecuritySELinuxSetFilecon(tmppath, shmem_seclabel->label);
> + else
> + return virSecuritySELinuxSetFilecon(tmppath, data->file_context);
> +}
> +
> +
> +static int
> virSecuritySELinuxSetSecurityAllLabel(virSecurityManagerPtr mgr,
> virDomainDefPtr def,
> const char *stdin_path)
> @@ -2549,4 +2616,7 @@ virSecurityDriver virSecurityDriverSELinux = {
>
> .domainGetSecurityMountOptions =
virSecuritySELinuxGetSecurityMountOptions,
> .getBaseLabel = virSecuritySELinuxGetBaseLabel,
> +
> + .domainSetSecurityShmemLabel = virSecuritySELinuxSetShmemLabel,
> + .domainRestoreSecurityShmemLabel = virSecuritySELinuxRestoreShmemLabel,
> };
> diff --git a/src/security/security_stack.c b/src/security/security_stack.c
> index 1ded57b..22c1b56 100644
> --- a/src/security/security_stack.c
> +++ b/src/security/security_stack.c
> @@ -599,6 +599,44 @@ virSecurityStackRestoreSecurityImageLabel(virSecurityManagerPtr
mgr,
> return rc;
> }
>
> +static int
> +virSecurityStackSetShmemLabel(virSecurityManagerPtr mgr,
> + virDomainDefPtr vm,
> + virDomainShmemDefPtr shmem,
> + char *path)
> +{
> + virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr);
> + virSecurityStackItemPtr item = priv->itemsHead;
> + int rc = 0;
> +
> + for (; item; item = item->next) {
> + if (virSecurityManagerSetShmemLabel(item->securityManager,
> + vm, shmem, path) < 0)
> + rc = -1;
> + }
> +
> + return rc;
> +}
> +
> +static int
> +virSecurityStackRestoreShmemLabel(virSecurityManagerPtr mgr,
> + virDomainDefPtr vm,
> + virDomainShmemDefPtr shmem,
> + char *path)
> +{
> + virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr);
> + virSecurityStackItemPtr item = priv->itemsHead;
> + int rc = 0;
> +
> + for (; item; item = item->next) {
> + if (virSecurityManagerRestoreShmemLabel(item->securityManager,
> + vm, shmem, path) < 0)
> + rc = -1;
> + }
> +
> + return rc;
> +}
> +
> virSecurityDriver virSecurityDriverStack = {
> .privateDataLen = sizeof(virSecurityStackData),
> .name = "stack",
> @@ -648,4 +686,7 @@ virSecurityDriver virSecurityDriverStack = {
> .domainSetSecurityHugepages = virSecurityStackSetHugepages,
>
> .getBaseLabel = virSecurityStackGetBaseLabel,
> +
> + .domainSetSecurityShmemLabel = virSecurityStackSetShmemLabel,
> + .domainRestoreSecurityShmemLabel = virSecurityStackRestoreShmemLabel,
> };
> --
> 1.8.3.1
Shouldn't it be implemented for the nop virSecurityDriver too? (note:
I don't know what it is for)
Good catch, i didn't notice that, i will add it in next version.
Luyao
> --
> libvir-list mailing list
> libvir-list(a)redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
4:54 a.m.
New subject: [libvirt] [PATCH 2/4] security: add security part for shmem device
On Thu, Jul 23, 2015 at 06:13:47PM +0800, Luyao Huang wrote:
A new api to help set/restore the shmem deivce dac/selinux label.
s/deivce/device/
Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
---
src/libvirt_private.syms | 2 ++
src/security/security_dac.c | 67 +++++++++++++++++++++++++++++++++++++++
src/security/security_driver.h | 11 +++++++
src/security/security_manager.c | 38 ++++++++++++++++++++++
src/security/security_manager.h | 8 +++++
src/security/security_selinux.c | 70 +++++++++++++++++++++++++++++++++++++++++
src/security/security_stack.c | 41 ++++++++++++++++++++++++
7 files changed, 237 insertions(+)
Also need to add to the security_nop.c impl
diff --git a/src/security/security_dac.c
b/src/security/security_dac.c
index deb6980..f954aa5 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -39,6 +39,7 @@
#include "virstoragefile.h"
#include "virstring.h"
#include "virutil.h"
+#include "virshm.h"
#define VIR_FROM_THIS VIR_FROM_SECURITY
@@ -922,6 +923,69 @@ virSecurityDACRestoreSecurityTPMFileLabel(virSecurityManagerPtr
mgr,
static int
+virSecurityDACSetShmemLabel(virSecurityManagerPtr mgr,
+ virDomainDefPtr def,
+ virDomainShmemDefPtr shmem,
+ char *path)
+{
+ virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
+ virSecurityLabelDefPtr seclabel;
+ virSecurityDeviceLabelDefPtr shmem_seclabel = NULL;
+ char *tmppath;
+ uid_t user;
+ gid_t group;
+
+ if (shmem->server.enabled)
+ tmppath = shmem->server.chr.data.nix.path;
+ else
+ tmppath = path;
Even when the server is enabled, QEMU still needs access to the path
doesn't it.
+
+ if (!tmppath)
+ return 0;
+
+ shmem_seclabel = virDomainShmemDefGetSecurityLabelDef(shmem, SECURITY_DAC_NAME);
+
+ if (shmem_seclabel && !shmem_seclabel->relabel)
+ return 0;
+
+ seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME);
+
+ if (shmem_seclabel && shmem_seclabel->label) {
+ if (virParseOwnershipIds(shmem_seclabel->label, &user, &group) <
0)
+ return -1;
+ } else {
+ if (virSecurityDACGetIds(seclabel, priv, &user, &group, NULL, NULL) <
0)
+ return -1;
+ }
+
+ return virSecurityDACSetOwnership(tmppath, user, group);
+}
+
+
+static int
+virSecurityDACRestoreShmemLabel(virSecurityManagerPtr mgr,
+ virDomainDefPtr def,
+ virDomainShmemDefPtr shmem,
+ char *path)
+{
+ virSecurityDeviceLabelDefPtr shmem_seclabel = NULL;
+
+ shmem_seclabel = virDomainShmemDefGetSecurityLabelDef(shmem, SECURITY_DAC_NAME);
+
+ if (shmem_seclabel && !shmem_seclabel->relabel)
+ return 0;
+
+ if (shmem->server.enabled)
+ return virSecurityDACRestoreChardevLabel(mgr, def, NULL,
&shmem->server.chr);
We need to restore path, even when server is enabled
+
+ if (!path)
+ return 0;
+
+ return virSecurityDACRestoreSecurityFileLabel(path);
+}
+
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 6e67a86..cbf89ee 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -46,6 +46,7 @@
#include "virconf.h"
#include "virtpm.h"
#include "virstring.h"
+#include "virshm.h"
#define VIR_FROM_THIS VIR_FROM_SECURITY
@@ -1888,6 +1889,37 @@ virSecuritySELinuxRestoreSecuritySmartcardCallback(virDomainDefPtr
def,
}
+static int
+virSecuritySELinuxRestoreShmemLabel(virSecurityManagerPtr mgr,
+ virDomainDefPtr def,
+ virDomainShmemDefPtr shmem,
+ char *path)
+{
+ char *tmppath = NULL;
+ virSecurityLabelDefPtr seclabel;
+ virSecurityDeviceLabelDefPtr shmem_seclabel = NULL;
+
+ seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME);
+ if (!seclabel || !seclabel->relabel)
+ return 0;
+
+ shmem_seclabel = virDomainShmemDefGetSecurityLabelDef(shmem,
SECURITY_SELINUX_NAME);
+
+ if (shmem_seclabel && !shmem_seclabel->relabel)
+ return 0;
+
+ if (shmem->server.enabled)
+ tmppath = shmem->server.chr.data.nix.path;
+ else
+ tmppath = path;
Same comment as earlier
+
+ if (!tmppath)
+ return 0;
+
+ return virSecuritySELinuxRestoreSecurityFileLabel(mgr, tmppath);
+}
+
+
static const char *
virSecuritySELinuxGetBaseLabel(virSecurityManagerPtr mgr, int virtType)
{
@@ -2284,6 +2316,41 @@ virSecuritySELinuxSetSecuritySmartcardCallback(virDomainDefPtr
def,
static int
+virSecuritySELinuxSetShmemLabel(virSecurityManagerPtr mgr,
+ virDomainDefPtr def,
+ virDomainShmemDefPtr shmem,
+ char *path)
+{
+ virSecuritySELinuxDataPtr data = virSecurityManagerGetPrivateData(mgr);
+ char *tmppath = NULL;
+ virSecurityLabelDefPtr seclabel;
+ virSecurityDeviceLabelDefPtr shmem_seclabel = NULL;
+
+ seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME);
+ if (!seclabel || !seclabel->relabel)
+ return 0;
+
+ shmem_seclabel = virDomainShmemDefGetSecurityLabelDef(shmem,
SECURITY_SELINUX_NAME);
+
+ if (shmem_seclabel && !shmem_seclabel->relabel)
+ return 0;
+
+ if (shmem->server.enabled)
+ tmppath = shmem->server.chr.data.nix.path;
+ else
+ tmppath = path;
And again
+
+ if (!tmppath)
+ return 0;
+
+ if (shmem_seclabel && shmem_seclabel->label)
+ return virSecuritySELinuxSetFilecon(tmppath, shmem_seclabel->label);
+ else
+ return virSecuritySELinuxSetFilecon(tmppath, data->file_context);
+}
+
+
+static int
virSecuritySELinuxSetSecurityAllLabel(virSecurityManagerPtr mgr,
virDomainDefPtr def,
const char *stdin_path)
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 :|
6:27 a.m.
New subject: [libvirt] [PATCH 2/4] security: add security part for shmem device
On 07/30/2015 05:54 PM, Daniel P. Berrange wrote:
On Thu, Jul 23, 2015 at 06:13:47PM +0800, Luyao Huang wrote:
> A new api to help set/restore the shmem deivce dac/selinux label.
s/deivce/device/
Thanks, i will fix in next version
> Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
> ---
> src/libvirt_private.syms | 2 ++
> src/security/security_dac.c | 67 +++++++++++++++++++++++++++++++++++++++
> src/security/security_driver.h | 11 +++++++
> src/security/security_manager.c | 38 ++++++++++++++++++++++
> src/security/security_manager.h | 8 +++++
> src/security/security_selinux.c | 70 +++++++++++++++++++++++++++++++++++++++++
> src/security/security_stack.c | 41 ++++++++++++++++++++++++
> 7 files changed, 237 insertions(+)
Also need to add to the security_nop.c impl
Okay, i must missed this file at that time :)
> diff --git a/src/security/security_dac.c
b/src/security/security_dac.c
> index deb6980..f954aa5 100644
> --- a/src/security/security_dac.c
> +++ b/src/security/security_dac.c
> @@ -39,6 +39,7 @@
> #include "virstoragefile.h"
> #include "virstring.h"
> #include "virutil.h"
> +#include "virshm.h"
>
> #define VIR_FROM_THIS VIR_FROM_SECURITY
>
> @@ -922,6 +923,69 @@ virSecurityDACRestoreSecurityTPMFileLabel(virSecurityManagerPtr
mgr,
>
>
> static int
> +virSecurityDACSetShmemLabel(virSecurityManagerPtr mgr,
> + virDomainDefPtr def,
> + virDomainShmemDefPtr shmem,
> + char *path)
> +{
> + virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
> + virSecurityLabelDefPtr seclabel;
> + virSecurityDeviceLabelDefPtr shmem_seclabel = NULL;
> + char *tmppath;
> + uid_t user;
> + gid_t group;
> +
> + if (shmem->server.enabled)
> + tmppath = shmem->server.chr.data.nix.path;
> + else
> + tmppath = path;
Even when the server is enabled, QEMU still needs access to the path
doesn't it.
...
> + if (shmem->server.enabled)
> + return virSecurityDACRestoreChardevLabel(mgr, def, NULL,
&shmem->server.chr);
We need to restore path, even when server is enabled
...
Same comment as earlier
...
+
+ if (shmem->server.enabled)
+ tmppath = shmem->server.chr.data.nix.path;
+ else
+ tmppath = path;
And again
Hmm...these $path are the shm path, if the shmem device is server
enabled, the used shm is created by ivshmem-server, which will depends
on ivshmem-server's label. And qemu process will connect to the socket
created by ivshmem-server, so change the label for shm created by
ivshmem-sever is useless (also we don't know the shm name created by
ivshmem-server).
However, never mind, i will do a refactor and remove the variable $path
here to make this function easy to put in
virSecuritySELinuxSetSecurityAllLabel and
virSecuritySELinuxSetSecurityAllLabel, since i noticed another reply
from you for this patch.
> +
> + if (!tmppath)
> + return 0;
> +
> + if (shmem_seclabel && shmem_seclabel->label)
> + return virSecuritySELinuxSetFilecon(tmppath, shmem_seclabel->label);
> + else
> + return virSecuritySELinuxSetFilecon(tmppath, data->file_context);
> +}
> +
> +
> +static int
> virSecuritySELinuxSetSecurityAllLabel(virSecurityManagerPtr mgr,
> virDomainDefPtr def,
> const char *stdin_path)
Regards,
Daniel
Thanks a lot for your review and help.
Luyao
5 a.m.
New subject: [libvirt] [PATCH 2/4] security: add security part for shmem device
On Thu, Jul 23, 2015 at 06:13:47PM +0800, Luyao Huang wrote:
A new api to help set/restore the shmem deivce dac/selinux label.
Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
---
src/libvirt_private.syms | 2 ++
src/security/security_dac.c | 67 +++++++++++++++++++++++++++++++++++++++
src/security/security_driver.h | 11 +++++++
src/security/security_manager.c | 38 ++++++++++++++++++++++
src/security/security_manager.h | 8 +++++
src/security/security_selinux.c | 70 +++++++++++++++++++++++++++++++++++++++++
src/security/security_stack.c | 41 ++++++++++++++++++++++++
7 files changed, 237 insertions(+)
diff --git a/src/security/security_dac.c
b/src/security/security_dac.c
index deb6980..f954aa5 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -1433,4 +1497,7 @@ virSecurityDriver virSecurityDriverDAC = {
.domainGetSecurityMountOptions = virSecurityDACGetMountOptions,
.getBaseLabel = virSecurityDACGetBaseLabel,
+
+ .domainSetSecurityShmemLabel = virSecurityDACSetShmemLabel,
+ .domainRestoreSecurityShmemLabel = virSecurityDACRestoreShmemLabel,
NB, you should also be modifying the virSecurityDACRestoreSecurityAllLabel
and virSecurityDACSetSecurityAllLabel methods to call this code during
diff --git a/src/security/security_selinux.c
b/src/security/security_selinux.c
index 6e67a86..cbf89ee 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -2549,4 +2616,7 @@ virSecurityDriver virSecurityDriverSELinux = {
.domainGetSecurityMountOptions = virSecuritySELinuxGetSecurityMountOptions,
.getBaseLabel = virSecuritySELinuxGetBaseLabel,
+
+ .domainSetSecurityShmemLabel = virSecuritySELinuxSetShmemLabel,
+ .domainRestoreSecurityShmemLabel = virSecuritySELinuxRestoreShmemLabel,
};
Likewise virSecuritySELinuxRestoreSecurityAllLabel and
virSecuritySELinuxSetSecurityAllLabel
Doing this avoids the need to manually call these shmem specific
security methods during general guest startup/shutdown. They only
need to be called manually during hotplug/unplug.
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 :|
6:43 a.m.
New subject: [libvirt] [PATCH 2/4] security: add security part for shmem device
On 07/30/2015 06:00 PM, Daniel P. Berrange wrote:
On Thu, Jul 23, 2015 at 06:13:47PM +0800, Luyao Huang wrote:
> A new api to help set/restore the shmem deivce dac/selinux label.
>
> Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
> ---
> src/libvirt_private.syms | 2 ++
> src/security/security_dac.c | 67 +++++++++++++++++++++++++++++++++++++++
> src/security/security_driver.h | 11 +++++++
> src/security/security_manager.c | 38 ++++++++++++++++++++++
> src/security/security_manager.h | 8 +++++
> src/security/security_selinux.c | 70 +++++++++++++++++++++++++++++++++++++++++
> src/security/security_stack.c | 41 ++++++++++++++++++++++++
> 7 files changed, 237 insertions(+)
>
> diff --git a/src/security/security_dac.c b/src/security/security_dac.c
> index deb6980..f954aa5 100644
> --- a/src/security/security_dac.c
> +++ b/src/security/security_dac.c
> @@ -1433,4 +1497,7 @@ virSecurityDriver virSecurityDriverDAC = {
> .domainGetSecurityMountOptions = virSecurityDACGetMountOptions,
>
> .getBaseLabel = virSecurityDACGetBaseLabel,
> +
> + .domainSetSecurityShmemLabel = virSecurityDACSetShmemLabel,
> + .domainRestoreSecurityShmemLabel = virSecurityDACRestoreShmemLabel,
NB, you should also be modifying the virSecurityDACRestoreSecurityAllLabel
and virSecurityDACSetSecurityAllLabel methods to call this code during
> diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
> index 6e67a86..cbf89ee 100644
> --- a/src/security/security_selinux.c
> +++ b/src/security/security_selinux.c
> @@ -2549,4 +2616,7 @@ virSecurityDriver virSecurityDriverSELinux = {
>
> .domainGetSecurityMountOptions =
virSecuritySELinuxGetSecurityMountOptions,
> .getBaseLabel = virSecuritySELinuxGetBaseLabel,
> +
> + .domainSetSecurityShmemLabel = virSecuritySELinuxSetShmemLabel,
> + .domainRestoreSecurityShmemLabel = virSecuritySELinuxRestoreShmemLabel,
> };
Likewise virSecuritySELinuxRestoreSecurityAllLabel and
virSecuritySELinuxSetSecurityAllLabel
Doing this avoids the need to manually call these shmem specific
security methods during general guest startup/shutdown. They only
need to be called manually during hotplug/unplug.
Okay, i see, i move these function in
virSecuritySELinuxRestoreSecurityAllLabel and
virSecuritySELinuxSetSecurityAllLabel.
Thanks a lot for your review and advise.
Regards,
Daniel
Luyao
5:13 a.m.
New subject: [libvirt] [PATCH 3/4] util: introduce new helpers to manage shmem device
Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
---
configure.ac | 10 +
po/POTFILES.in | 3 +-
src/Makefile.am | 5 +-
src/libvirt_private.syms | 16 ++
src/util/virshm.c | 623 +++++++++++++++++++++++++++++++++++++++++++++++
src/util/virshm.h | 104 ++++++++
6 files changed, 759 insertions(+), 2 deletions(-)
create mode 100644 src/util/virshm.c
create mode 100644 src/util/virshm.h
diff --git a/configure.ac b/configure.ac
index a7f38e8..940eb66 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1176,6 +1176,16 @@ if test "$with_linux" = "yes"; then
]])
fi
+dnl
+dnl check for POSIX share memory functions
+dnl
+LIBRT_LIBS=""
+AC_CHECK_LIB([rt],[shm_open],[LIBRT_LIBS="-lrt"])
+old_libs="$LIBS"
+LIBS="$old_libs $LIBRT_LIBS"
+AC_CHECK_FUNCS([shm_open])
+LIBS="$old_libs"
+AC_SUBST([LIBRT_LIBS])
dnl Need to test if pkg-config exists
PKG_PROG_PKG_CONFIG
diff --git a/po/POTFILES.in b/po/POTFILES.in
index a75f5ae..7687a82 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -215,8 +215,9 @@ src/util/virpolkit.c
src/util/virportallocator.c
src/util/virprocess.c
src/util/virrandom.c
-src/util/virsexpr.c
src/util/virscsi.c
+src/util/virsexpr.c
+src/util/virshm.c
src/util/virsocketaddr.c
src/util/virstats.c
src/util/virstorageencryption.c
diff --git a/src/Makefile.am b/src/Makefile.am
index 7338ab9..048a096 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -152,6 +152,7 @@ UTIL_SOURCES = \
util/virscsi.c util/virscsi.h \
util/virseclabel.c util/virseclabel.h \
util/virsexpr.c util/virsexpr.h \
+ util/virshm.c util/virshm.h \
util/virsocketaddr.h util/virsocketaddr.c \
util/virstats.c util/virstats.h \
util/virstorageencryption.c util/virstorageencryption.h \
@@ -1048,7 +1049,7 @@ libvirt_util_la_LIBADD = $(CAPNG_LIBS) $(YAJL_LIBS) $(LIBNL_LIBS) \
$(THREAD_LIBS) $(AUDIT_LIBS) $(DEVMAPPER_LIBS) \
$(LIB_CLOCK_GETTIME) $(DBUS_LIBS) $(MSCOM_LIBS) $(LIBXML_LIBS) \
$(SECDRIVER_LIBS) $(NUMACTL_LIBS) $(SYSTEMD_DAEMON_LIBS) \
- $(POLKIT_LIBS)
+ $(POLKIT_LIBS) $(LIBRT_LIBS)
noinst_LTLIBRARIES += libvirt_conf.la
@@ -1284,6 +1285,7 @@ libvirt_driver_qemu_impl_la_LIBADD = $(CAPNG_LIBS) \
$(GNUTLS_LIBS) \
$(LIBNL_LIBS) \
$(LIBXML_LIBS) \
+ $(LIBRT_LIBS) \
$(NULL)
libvirt_driver_qemu_impl_la_SOURCES = $(QEMU_DRIVER_SOURCES)
@@ -2264,6 +2266,7 @@ libvirt_setuid_rpc_client_la_LDFLAGS = \
$(AM_LDFLAGS) \
$(LIBXML_LIBS) \
$(SECDRIVER_LIBS) \
+ $(LIBRT_LIBS) \
$(NULL)
libvirt_setuid_rpc_client_la_CFLAGS = \
-DLIBVIRT_SETUID_RPC_CLIENT \
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index af73177..977fd34 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2078,6 +2078,22 @@ sexpr_u64;
string2sexpr;
+# util/virshm.h
+virShmBuildPath;
+virShmCreate;
+virShmObjectFree;
+virShmObjectNew;
+virShmObjectListAdd;
+virShmObjectListDel;
+virShmObjectFindByName;
+virShmObjectListGetDefault;
+virShmObjectRemoveStateFile;
+virShmObjectSaveState;
+virShmOpen;
+virShmRemoveUsedDomain;
+virShmSetUsedDomain;
+virShmUnlink;
+
# util/virsocketaddr.h
virSocketAddrBroadcast;
virSocketAddrBroadcastByPrefix;
diff --git a/src/util/virshm.c b/src/util/virshm.c
new file mode 100644
index 0000000..7ab39be
--- /dev/null
+++ b/src/util/virshm.c
@@ -0,0 +1,623 @@
+/*
+ * virshm.c: helper API for POSIX share memory
+ *
+ * Copyright (C) 2015 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see
+ * <http://www.gnu.org/licenses/>;.
+ *
+ */
+
+#include <config.h>
+
+#include <fcntl.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+
+#ifdef HAVE_SHM_OPEN
+# include <sys/mman.h>
+#endif
+
+#include "virshm.h"
+#include "virxml.h"
+#include "virbuffer.h"
+#include "virerror.h"
+#include "virstring.h"
+#include "virlog.h"
+#include "virutil.h"
+#include "viralloc.h"
+#include "virfile.h"
+#include "configmake.h"
+
+#define VIR_FROM_THIS VIR_FROM_NONE
+
+VIR_LOG_INIT("util.shm");
+
+#define SHMEM_STATE_DIR LOCALSTATEDIR "/run/libvirt/shmem"
+
+VIR_ENUM_IMPL(virShmObject, VIR_SHM_TYPE_LAST,
+ "shm",
+ "server");
+
+static virClassPtr virShmObjectListClass;
+
+static virShmObjectListPtr mainlist; /* global shm object list */
+
+static void virShmObjectListDispose(void *obj);
+
+static int
+virShmObjectListLoadState(virShmObjectPtr *shmobj,
+ const char *stateDir,
+ const char *name)
+{
+ char *stateFile = NULL;
+ xmlXPathContextPtr ctxt = NULL;
+ xmlDocPtr xml = NULL;
+ virShmObjectPtr tmpshm;
+ xmlNodePtr *usagenode = NULL;
+ xmlNodePtr save = NULL;
+ int ret = -1;
+ char *drivername = NULL;
+ char *shmtype = NULL;
+ int nusages;
+
+ if (VIR_ALLOC(tmpshm) < 0)
+ return -1;
+
+ if (!(stateFile = virFileBuildPath(stateDir, name, ".xml")))
+ goto error;
+
+ if (!(xml = virXMLParseFileCtxt(stateFile, &ctxt)))
+ goto error;
+
+ tmpshm->name = virXPathString("string(./name)", ctxt);
+ if (!tmpshm->name) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("shmem missing name attribute"));
+ goto error;
+ }
+
+ shmtype = virXPathString("string(./type)", ctxt);
+ if (!shmtype) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("shmem missing type attribute"));
+ goto error;
+ }
+ if ((tmpshm->type = virShmObjectTypeFromString(shmtype)) < 0) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("invalid shmem object type %s"), shmtype);
+ goto error;
+ }
+
+ if (virXPathULongLong("string(./size)", ctxt, &tmpshm->size) < 0)
{
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("shmem missing or have invalid size attribute"));
+ goto error;
+ }
+
+ tmpshm->path = virXPathString("string(./path)", ctxt);
+ if (virXPathBoolean("boolean(./othercreate)", ctxt))
+ tmpshm->othercreate = true;
+
+ if (!(drivername = virXPathString("string(./@driver)", ctxt))) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("shmem usage element missing driver attribute"));
+ goto error;
+ }
+ nusages = virXPathNodeSet("./domain", ctxt, &usagenode);
+ if (nusages < 0)
+ goto error;
+
+ if (nusages > 0) {
+ size_t i;
+
+ for (i = 0; i < nusages; i++) {
+ char *domainname;
+
+ save = ctxt->node;
+ ctxt->node = usagenode[i];
+
+ if (!(domainname = virXPathString("string(./@name)", ctxt))) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("shmem domain element missing name
attribute"));
+ goto error;
+ }
+
+ if (virShmSetUsedDomain(tmpshm, drivername, domainname) < 0) {
+ VIR_FREE(domainname);
+ goto error;
+ }
+ VIR_FREE(domainname);
+ ctxt->node = save;
+ }
+ }
+ *shmobj = tmpshm;
+ ret = 0;
+ cleanup:
+ VIR_FREE(stateFile);
+ VIR_FREE(drivername);
+ VIR_FREE(shmtype);
+ xmlFreeDoc(xml);
+ xmlXPathFreeContext(ctxt);
+ return ret;
+
+ error:
+ virShmObjectFree(tmpshm);
+ goto cleanup;
+}
+
+static int
+virShmObjectListLoadAllState(virShmObjectListPtr list)
+{
+ DIR *dir;
+ struct dirent *entry;
+
+ if (!(dir = opendir(list->stateDir))) {
+ if (errno == ENOENT)
+ return 0;
+ virReportSystemError(errno, _("Failed to open dir '%s'"),
list->stateDir);
+ return -1;
+ }
+
+ while (virDirRead(dir, &entry, list->stateDir) > 0) {
+ virShmObjectPtr shmobj;
+
+ if (entry->d_name[0] == '.' ||
+ !virFileStripSuffix(entry->d_name, ".xml"))
+ continue;
+ if (virShmObjectListLoadState(&shmobj, list->stateDir, entry->d_name)
< 0)
+ continue;
+ if (virShmObjectListAdd(list, shmobj) < 0)
+ continue;
+ }
+ closedir(dir);
+ return 0;
+}
+
+static virShmObjectListPtr
+virShmObjectListNew(void)
+{
+ virShmObjectListPtr list;
+ bool privileged = geteuid() == 0;
+
+ if (!(list = virObjectLockableNew(virShmObjectListClass)))
+ return NULL;
+
+ virObjectLock(list);
+ if (privileged) {
+ if (VIR_STRDUP(list->stateDir, SHMEM_STATE_DIR) < 0)
+ goto error;
+ } else {
+ char *rundir = NULL;
+
+ if (!(rundir = virGetUserRuntimeDirectory()))
+ goto error;
+
+ if (virAsprintf(&list->stateDir, "%s/shmem", rundir) < 0) {
+ VIR_FREE(rundir);
+ goto error;
+ }
+ VIR_FREE(rundir);
+ }
+ if (virFileMakePath(list->stateDir) < 0) {
+ virReportError(VIR_ERR_OPERATION_FAILED,
+ _("Failed to create state dir '%s'"),
+ list->stateDir);
+ goto error;
+ }
+ if (virShmObjectListLoadAllState(list) < 0)
+ goto error;
+
+ virObjectUnlock(list);
+ return list;
+
+ error:
+ virObjectUnlock(list);
+ virObjectUnref(list);
+ return NULL;
+}
+
+static int
+virShmOnceInit(void)
+{
+ if (!(virShmObjectListClass = virClassNew(virClassForObjectLockable(),
+ "virShmObjectList",
+ sizeof(virShmObjectList),
+ virShmObjectListDispose)))
+ return -1;
+
+ if (!(mainlist = virShmObjectListNew()))
+ return -1;
+
+ return 0;
+}
+
+VIR_ONCE_GLOBAL_INIT(virShm)
+
+virShmObjectListPtr
+virShmObjectListGetDefault(void)
+{
+ if (virShmInitialize() < 0)
+ return NULL;
+
+ return virObjectRef(mainlist);
+}
+
+int
+virShmSetUsedDomain(virShmObjectPtr shmobj,
+ const char *drvname,
+ const char *domname)
+{
+ char *tmpdomain = NULL;
+
+ if (shmobj->drvname) {
+ if (STRNEQ(drvname, shmobj->drvname)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("cannot use one shmem for different driver"));
+ goto error;
+ }
+ } else {
+ if (VIR_STRDUP(shmobj->drvname, drvname) < 0)
+ goto error;
+ }
+
+ if (VIR_STRDUP(tmpdomain, domname) < 0)
+ goto error;
+
+ if (VIR_APPEND_ELEMENT(shmobj->domains, shmobj->ndomains, tmpdomain) < 0)
+ goto error;
+
+ return 0;
+
+ error:
+ VIR_FREE(tmpdomain);
+ return -1;
+}
+
+void
+virShmObjectFree(virShmObjectPtr shmobj)
+{
+ size_t i;
+
+ if (!shmobj)
+ return;
+
+ VIR_FREE(shmobj->name);
+ VIR_FREE(shmobj->path);
+ VIR_FREE(shmobj->drvname);
+ for (i = 0; i < shmobj->ndomains; i++)
+ VIR_FREE(shmobj->domains[i]);
+ VIR_FREE(shmobj->domains);
+ VIR_FREE(shmobj);
+}
+
+virShmObjectPtr
+virShmObjectNew(const char *name,
+ unsigned long long size,
+ const char *path,
+ int type,
+ bool othercreate,
+ const char *drvname,
+ const char *domname)
+{
+ virShmObjectPtr shmobj;
+
+ if (VIR_ALLOC(shmobj) < 0)
+ return NULL;
+
+ shmobj->size = size;
+ shmobj->type = type;
+
+ if (VIR_STRDUP(shmobj->name, name) < 0)
+ goto error;
+
+ if (path) {
+ if (VIR_STRDUP(shmobj->path, path) < 0)
+ goto error;
+ } else {
+ VIR_FREE(shmobj->path);
+ }
+ shmobj->othercreate = othercreate;
+
+ if (virShmSetUsedDomain(shmobj, drvname, domname) < 0)
+ goto error;
+
+ return shmobj;
+
+ error:
+ virShmObjectFree(shmobj);
+ return NULL;
+}
+
+static void
+virShmObjectListDispose(void *obj)
+{
+ virShmObjectListPtr list = obj;
+ size_t i;
+
+ for (i = 0; i < list->nshmobjs; i++)
+ virShmObjectFree(list->shmobjs[i]);
+
+ VIR_FREE(list->shmobjs);
+}
+
+int
+virShmObjectSaveState(virShmObjectPtr shmobj,
+ const char *stateDir)
+{
+ virBuffer buf = VIR_BUFFER_INITIALIZER;
+ int ret = -1;
+ size_t i;
+ char *xml = NULL;
+ char *stateFile = NULL;
+
+ virBufferAddLit(&buf, "<shmem>\n");
+ virBufferAdjustIndent(&buf, 2);
+
+ virBufferAsprintf(&buf, "<name>%s</name>\n",
shmobj->name);
+
+ virBufferAsprintf(&buf, "<type>%s</type>\n",
virShmObjectTypeToString(shmobj->type));
+
+ virBufferAsprintf(&buf, "<size>%llu</size>\n",
shmobj->size);
+ if (shmobj->path)
+ virBufferAsprintf(&buf, "<path>%s</path>\n",
shmobj->path);
+ if (shmobj->othercreate)
+ virBufferAddLit(&buf, "<othercreate/>\n");
+ virBufferAsprintf(&buf, "<driver>%s</driver>\n",
shmobj->drvname);
+ for (i = 0; i < shmobj->ndomains; i++)
+ virBufferAsprintf(&buf, "<domain name='%s'/>\n",
shmobj->domains[i]);
+
+ virBufferAdjustIndent(&buf, -2);
+ virBufferAddLit(&buf, "</shmem>\n");
+
+ if (virBufferCheckError(&buf) < 0)
+ goto cleanup;
+
+ if (!(xml = virBufferContentAndReset(&buf)))
+ goto cleanup;
+
+ if (!(stateFile = virFileBuildPath(stateDir, shmobj->name, ".xml")))
+ goto cleanup;
+
+ if (virXMLSaveFile(stateFile, NULL, NULL, xml) < 0)
+ goto cleanup;
+
+ ret = 0;
+
+ cleanup:
+ VIR_FREE(stateFile);
+ VIR_FREE(xml);
+ return ret;
+}
+
+int
+virShmRemoveUsedDomain(virShmObjectPtr shmobj,
+ const char *drvname,
+ const char *domname)
+{
+ size_t i;
+
+ if (shmobj->drvname) {
+ if (STRNEQ(drvname, shmobj->drvname)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("cannot use one shmem for different driver"));
+ return -1;
+ }
+ } else {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("cannot get shmem object driver name"));
+ return -1;
+ }
+
+ for (i = 0; i < shmobj->ndomains; i++) {
+ if (STREQ(shmobj->domains[i], domname)) {
+ VIR_FREE(shmobj->domains[i]);
+ VIR_DELETE_ELEMENT(shmobj->domains, i, shmobj->ndomains);
+ return 0;
+ }
+ }
+
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("cannot find domname in shmem object domain list"));
+ return -1;
+}
+
+int
+virShmObjectRemoveStateFile(virShmObjectListPtr list,
+ const char *name)
+{
+ char *stateFile = NULL;
+
+ if (!(stateFile = virFileBuildPath(list->stateDir, name, ".xml")))
+ return -1;
+ unlink(stateFile);
+ VIR_FREE(stateFile);
+ return 0;
+}
+
+int
+virShmObjectListDel(virShmObjectListPtr list,
+ virShmObjectPtr shmobj)
+{
+ size_t i;
+
+ for (i = 0; i < list->nshmobjs; i++) {
+ virShmObjectPtr tmp = list->shmobjs[i];
+
+ if (STREQ(tmp->name, shmobj->name)) {
+ VIR_DELETE_ELEMENT(list->shmobjs, i, list->nshmobjs);
+ return 0;
+ }
+ }
+ return -1;
+}
+
+virShmObjectPtr
+virShmObjectFindByName(virShmObjectListPtr list,
+ const char *name)
+{
+ size_t i;
+
+ for (i = 0; i < list->nshmobjs; i++) {
+ if (STREQ_NULLABLE(list->shmobjs[i]->name, name))
+ return list->shmobjs[i];
+ }
+
+ return NULL;
+}
+
+int
+virShmObjectListAdd(virShmObjectListPtr list,
+ virShmObjectPtr shmobj)
+{
+ if (virShmObjectFindByName(list, shmobj->name)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("A share memory object named '%s' already
exists"),
+ shmobj->name);
+ return -1;
+ }
+
+ return VIR_APPEND_ELEMENT(list->shmobjs, list->nshmobjs, shmobj);
+}
+
+#ifdef HAVE_SHM_OPEN
+int
+virShmOpen(const char *name,
+ unsigned long long size,
+ mode_t mode)
+{
+ int fd = -1;
+ struct stat sb;
+
+ if ((fd = shm_open(name, O_RDWR, mode)) < 0) {
+ virReportSystemError(errno,
+ _("Unable to open shared memory"
+ " objects '%s'"),
+ name);
+ return -1;
+ }
+
+ if (fstat(fd, &sb) < 0) {
+ virReportSystemError(errno,
+ _("cannot stat file '%s'"), name);
+ goto error;
+ }
+
+ if (sb.st_size < size) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("already exist shared memory object"
+ " size %ju is smaller than required size %llu"),
+ (uintmax_t)sb.st_size, size);
+ goto error;
+ }
+
+ return fd;
+
+ error:
+ VIR_FORCE_CLOSE(fd);
+ return -1;
+}
+
+int
+virShmCreate(const char *name,
+ unsigned long long size,
+ bool outerr,
+ bool *othercreate,
+ mode_t mode)
+{
+ int fd = -1;
+
+ fd = shm_open(name, O_RDWR|O_CREAT|O_EXCL, mode);
+
+ if (fd > 0) {
+ if (ftruncate(fd, size) != 0) {
+ virReportSystemError(errno,
+ _("Unable to truncate"
+ " file descriptor to %llu"),
+ size);
+ ignore_value(shm_unlink(name));
+ VIR_FORCE_CLOSE(fd);
+ return -1;
+ }
+ } else if (errno == EEXIST) {
+ if (outerr) {
+ virReportSystemError(errno,
+ _("shared memory objects"
+ " '%s' is already exist"),
+ name);
+ return -1;
+ }
+
+ VIR_WARN("shared memory objects '%s' is already exist", name);
+ *othercreate = true;
+
+ return virShmOpen(name, size, mode);
+ } else {
+ virReportSystemError(errno,
+ _("Unable to create shared memory"
+ " objects '%s'"),
+ name);
+ return -1;
+ }
+
+ return fd;
+}
+
+int
+virShmUnlink(const char *name)
+{
+ int ret;
+
+ if ((ret = shm_unlink(name)) < 0) {
+ virReportSystemError(errno,
+ _("Unable to delete shared memory"
+ " objects '%s'"),
+ name);
+ }
+ return ret;
+}
+
+# if defined(__linux__)
+# define SHM_DEFAULT_PATH "/dev/shm"
+
+int
+virShmBuildPath(const char *name, char **path)
+{
+
+ if (virAsprintf(path, "%s/%s", SHM_DEFAULT_PATH, name) < 0)
+ return -1;
+
+ if (!virFileExists(*path)) {
+ virReportSystemError(errno,
+ _("could not access %s"),
+ *path);
+ VIR_FREE(*path);
+ return -1;
+ }
+ return 0;
+}
+
+# else
+
+int
+virShmBuildPath(const char *name ATTRIBUTE_UNUSED,
+ char **path ATTRIBUTE_UNUSED)
+{
+ virReportSystemError(ENOSYS, "%s",
+ _("Cannot get share memory object path on this
platform"));
+ return -2;
+}
+
+# endif
+#endif /* HAVE_SHM_OPEN */
diff --git a/src/util/virshm.h b/src/util/virshm.h
new file mode 100644
index 0000000..945a541
--- /dev/null
+++ b/src/util/virshm.h
@@ -0,0 +1,104 @@
+/*
+ * virshm.h: helper API for POSIX share memory
+ *
+ * Copyright (C) 2015 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see
+ * <http://www.gnu.org/licenses/>;.
+ *
+ */
+
+#ifndef __VIR_SHM_H__
+# define __VIR_SHM_H__
+
+# include "internal.h"
+# include "virutil.h"
+# include "virobject.h"
+
+typedef enum {
+ VIR_SHM_TYPE_SHM = 0,
+ VIR_SHM_TYPE_SERVER,
+
+ VIR_SHM_TYPE_LAST
+} virShmObjectType;
+
+typedef struct _virShmObject virShmObject;
+typedef virShmObject *virShmObjectPtr;
+struct _virShmObject {
+ char *name; /* shmem object name */
+ int type; /* shmem object type */
+ unsigned long long size; /* size of shmem object */
+ char *path; /* shmem path */
+ bool othercreate; /* a bool parameter record if the shm is created by
libvirt */
+
+ char *drvname; /* which driver */
+ char **domains; /* domain(s) using this shm */
+ size_t ndomains; /* number of useds */
+};
+
+VIR_ENUM_DECL(virShmObject);
+
+typedef struct _virShmObjectList virShmObjectList;
+typedef virShmObjectList *virShmObjectListPtr;
+struct _virShmObjectList {
+ virObjectLockable parent;
+ char *stateDir;
+ size_t nshmobjs;
+ virShmObjectPtr *shmobjs;
+};
+
+virShmObjectListPtr virShmObjectListGetDefault(void);
+
+int virShmSetUsedDomain(virShmObjectPtr shmobj, const char *drvname, const char
*domname)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
+int virShmRemoveUsedDomain(virShmObjectPtr shmobj, const char *drvname, const char
*domname)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
+void virShmObjectFree(virShmObjectPtr shmobj);
+virShmObjectPtr virShmObjectNew(const char *name, unsigned long long size,
+ const char *path, int type, bool othercreate,
+ const char *drvname, const char *domname)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(6)
+ ATTRIBUTE_NONNULL(7);
+int virShmObjectSaveState(virShmObjectPtr shmobj, const char *stateDir)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+int virShmObjectRemoveStateFile(virShmObjectListPtr list, const char *name)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+int virShmObjectListAdd(virShmObjectListPtr list, virShmObjectPtr shmobj)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+int virShmObjectListDel(virShmObjectListPtr list, virShmObjectPtr shmobj)
+ ATTRIBUTE_NONNULL(1);
+virShmObjectPtr virShmObjectFindByName(virShmObjectListPtr list,
+ const char *name)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+
+# ifdef HAVE_SHM_OPEN
+int virShmCreate(const char *name, unsigned long long size,
+ bool outerr, bool *othercreate, mode_t mode)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(4);
+int virShmOpen(const char *name,
+ unsigned long long size,
+ mode_t mode)
+ ATTRIBUTE_NONNULL(1);
+int virShmUnlink(const char *name)
+ ATTRIBUTE_NONNULL(1);
+int virShmBuildPath(const char *name, char **path)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+# else /* HAVE_SHM_OPEN */
+# define virShmCreate(name, size, outerr, othercreate, mode) -2
+# define virShmOpen(name, size, mode) -2
+# define virShmUnlink(name) -2
+# define virShmBuildPath(name, path) -2
+
+# endif /* HAVE_SHM_OPEN */
+#endif /* __VIR_SHM_H__ */
--
1.8.3.1
10:43 a.m.
New subject: [libvirt] [PATCH 3/4] util: introduce new helpers to manage shmem device
Hi
On Thu, Jul 23, 2015 at 12:13 PM, Luyao Huang <lhuang(a)redhat.com> wrote:
Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
---
configure.ac | 10 +
po/POTFILES.in | 3 +-
src/Makefile.am | 5 +-
src/libvirt_private.syms | 16 ++
src/util/virshm.c | 623 +++++++++++++++++++++++++++++++++++++++++++++++
src/util/virshm.h | 104 ++++++++
6 files changed, 759 insertions(+), 2 deletions(-)
create mode 100644 src/util/virshm.c
create mode 100644 src/util/virshm.h
(would be nicer to have some tests)
After applying this patch, make check fails with
Symbol block at ./libvirt_private.syms:2081: symbols not sorted
virShmObjectFree
virShmObjectNew
virShmObjectListAdd
virShmObjectListDel
virShmObjectFindByName
virShmObjectListGetDefault
Correct ordering
virShmObjectFindByName
virShmObjectFree
virShmObjectListAdd
virShmObjectListDel
virShmObjectListGetDefault
virShmObjectNew
Makefile:10195: recipe for target 'check-symsorting' failed
diff --git a/configure.ac b/configure.ac
index a7f38e8..940eb66 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1176,6 +1176,16 @@ if test "$with_linux" = "yes"; then
]])
fi
+dnl
+dnl check for POSIX share memory functions
+dnl
+LIBRT_LIBS=""
+AC_CHECK_LIB([rt],[shm_open],[LIBRT_LIBS="-lrt"])
+old_libs="$LIBS"
+LIBS="$old_libs $LIBRT_LIBS"
+AC_CHECK_FUNCS([shm_open])
+LIBS="$old_libs"
+AC_SUBST([LIBRT_LIBS])
dnl Need to test if pkg-config exists
PKG_PROG_PKG_CONFIG
diff --git a/po/POTFILES.in b/po/POTFILES.in
index a75f5ae..7687a82 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -215,8 +215,9 @@ src/util/virpolkit.c
src/util/virportallocator.c
src/util/virprocess.c
src/util/virrandom.c
-src/util/virsexpr.c
src/util/virscsi.c
+src/util/virsexpr.c
+src/util/virshm.c
src/util/virsocketaddr.c
src/util/virstats.c
src/util/virstorageencryption.c
diff --git a/src/Makefile.am b/src/Makefile.am
index 7338ab9..048a096 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -152,6 +152,7 @@ UTIL_SOURCES =
\
util/virscsi.c util/virscsi.h \
util/virseclabel.c util/virseclabel.h \
util/virsexpr.c util/virsexpr.h \
+ util/virshm.c util/virshm.h \
util/virsocketaddr.h util/virsocketaddr.c \
util/virstats.c util/virstats.h \
util/virstorageencryption.c util/virstorageencryption.h \
@@ -1048,7 +1049,7 @@ libvirt_util_la_LIBADD = $(CAPNG_LIBS) $(YAJL_LIBS) $(LIBNL_LIBS)
\
$(THREAD_LIBS) $(AUDIT_LIBS) $(DEVMAPPER_LIBS) \
$(LIB_CLOCK_GETTIME) $(DBUS_LIBS) $(MSCOM_LIBS) $(LIBXML_LIBS) \
$(SECDRIVER_LIBS) $(NUMACTL_LIBS) $(SYSTEMD_DAEMON_LIBS) \
- $(POLKIT_LIBS)
+ $(POLKIT_LIBS) $(LIBRT_LIBS)
noinst_LTLIBRARIES += libvirt_conf.la
@@ -1284,6 +1285,7 @@ libvirt_driver_qemu_impl_la_LIBADD = $(CAPNG_LIBS) \
$(GNUTLS_LIBS) \
$(LIBNL_LIBS) \
$(LIBXML_LIBS) \
+ $(LIBRT_LIBS) \
$(NULL)
libvirt_driver_qemu_impl_la_SOURCES = $(QEMU_DRIVER_SOURCES)
@@ -2264,6 +2266,7 @@ libvirt_setuid_rpc_client_la_LDFLAGS = \
$(AM_LDFLAGS) \
$(LIBXML_LIBS) \
$(SECDRIVER_LIBS) \
+ $(LIBRT_LIBS) \
$(NULL)
libvirt_setuid_rpc_client_la_CFLAGS = \
-DLIBVIRT_SETUID_RPC_CLIENT \
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index af73177..977fd34 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2078,6 +2078,22 @@ sexpr_u64;
string2sexpr;
+# util/virshm.h
+virShmBuildPath;
+virShmCreate;
+virShmObjectFree;
+virShmObjectNew;
+virShmObjectListAdd;
+virShmObjectListDel;
+virShmObjectFindByName;
+virShmObjectListGetDefault;
+virShmObjectRemoveStateFile;
+virShmObjectSaveState;
+virShmOpen;
+virShmRemoveUsedDomain;
+virShmSetUsedDomain;
+virShmUnlink;
So these lines should be sorted
+
# util/virsocketaddr.h
virSocketAddrBroadcast;
virSocketAddrBroadcastByPrefix;
diff --git a/src/util/virshm.c b/src/util/virshm.c
new file mode 100644
index 0000000..7ab39be
--- /dev/null
+++ b/src/util/virshm.c
@@ -0,0 +1,623 @@
+/*
+ * virshm.c: helper API for POSIX share memory
+ *
+ * Copyright (C) 2015 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see
+ * <http://www.gnu.org/licenses/>;.
+ *
+ */
+
+#include <config.h>
+
+#include <fcntl.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+
+#ifdef HAVE_SHM_OPEN
+# include <sys/mman.h>
+#endif
+
+#include "virshm.h"
+#include "virxml.h"
+#include "virbuffer.h"
+#include "virerror.h"
+#include "virstring.h"
+#include "virlog.h"
+#include "virutil.h"
+#include "viralloc.h"
+#include "virfile.h"
+#include "configmake.h"
+
+#define VIR_FROM_THIS VIR_FROM_NONE
+
+VIR_LOG_INIT("util.shm");
+
+#define SHMEM_STATE_DIR LOCALSTATEDIR "/run/libvirt/shmem"
+
+VIR_ENUM_IMPL(virShmObject, VIR_SHM_TYPE_LAST,
+ "shm",
+ "server");
+
+static virClassPtr virShmObjectListClass;
+
+static virShmObjectListPtr mainlist; /* global shm object list */
+
+static void virShmObjectListDispose(void *obj);
+
+static int
+virShmObjectListLoadState(virShmObjectPtr *shmobj,
+ const char *stateDir,
+ const char *name)
+{
+ char *stateFile = NULL;
+ xmlXPathContextPtr ctxt = NULL;
+ xmlDocPtr xml = NULL;
+ virShmObjectPtr tmpshm;
+ xmlNodePtr *usagenode = NULL;
+ xmlNodePtr save = NULL;
save could be moved in the using scope
+ int ret = -1;
+ char *drivername = NULL;
+ char *shmtype = NULL;
+ int nusages;
+
+ if (VIR_ALLOC(tmpshm) < 0)
+ return -1;
+
+ if (!(stateFile = virFileBuildPath(stateDir, name, ".xml")))
+ goto error;
+
+ if (!(xml = virXMLParseFileCtxt(stateFile, &ctxt)))
+ goto error;
+
+ tmpshm->name = virXPathString("string(./name)", ctxt);
+ if (!tmpshm->name) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("shmem missing name attribute"));
+ goto error;
+ }
+
+ shmtype = virXPathString("string(./type)", ctxt);
+ if (!shmtype) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("shmem missing type attribute"));
+ goto error;
+ }
+ if ((tmpshm->type = virShmObjectTypeFromString(shmtype)) < 0) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("invalid shmem object type %s"), shmtype);
You leak shmtype here and in other goto error
+ goto error;
+ }
+
+ if (virXPathULongLong("string(./size)", ctxt, &tmpshm->size) <
0) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("shmem missing or have invalid size attribute"));
+ goto error;
+ }
+
+ tmpshm->path = virXPathString("string(./path)", ctxt);
+ if (virXPathBoolean("boolean(./othercreate)", ctxt))
+ tmpshm->othercreate = true;
+
+ if (!(drivername = virXPathString("string(./@driver)", ctxt))) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("shmem usage element missing driver attribute"));
+ goto error;
+ }
+ nusages = virXPathNodeSet("./domain", ctxt, &usagenode);
+ if (nusages < 0)
+ goto error;
+
+ if (nusages > 0) {
+ size_t i;
+
+ for (i = 0; i < nusages; i++) {
+ char *domainname;
+
+ save = ctxt->node;
+ ctxt->node = usagenode[i];
+
+ if (!(domainname = virXPathString("string(./@name)", ctxt))) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("shmem domain element missing name
attribute"));
+ goto error;
+ }
+
+ if (virShmSetUsedDomain(tmpshm, drivername, domainname) < 0) {
+ VIR_FREE(domainname);
+ goto error;
+ }
+ VIR_FREE(domainname);
+ ctxt->node = save;
+ }
+ }
+ *shmobj = tmpshm;
+ ret = 0;
+ cleanup:
+ VIR_FREE(stateFile);
+ VIR_FREE(drivername);
+ VIR_FREE(shmtype);
+ xmlFreeDoc(xml);
+ xmlXPathFreeContext(ctxt);
+ return ret;
+
+ error:
+ virShmObjectFree(tmpshm);
+ goto cleanup;
+}
+
+static int
+virShmObjectListLoadAllState(virShmObjectListPtr list)
+{
+ DIR *dir;
+ struct dirent *entry;
+
+ if (!(dir = opendir(list->stateDir))) {
+ if (errno == ENOENT)
+ return 0;
+ virReportSystemError(errno, _("Failed to open dir '%s'"),
list->stateDir);
+ return -1;
+ }
+
+ while (virDirRead(dir, &entry, list->stateDir) > 0) {
+ virShmObjectPtr shmobj;
+
+ if (entry->d_name[0] == '.' ||
+ !virFileStripSuffix(entry->d_name, ".xml"))
+ continue;
+ if (virShmObjectListLoadState(&shmobj, list->stateDir, entry->d_name)
< 0)
+ continue;
+ if (virShmObjectListAdd(list, shmobj) < 0)
+ continue;
+ }
+ closedir(dir);
+ return 0;
+}
+
+static virShmObjectListPtr
+virShmObjectListNew(void)
+{
+ virShmObjectListPtr list;
+ bool privileged = geteuid() == 0;
+
+ if (!(list = virObjectLockableNew(virShmObjectListClass)))
+ return NULL;
+
+ virObjectLock(list);
I am not sure the lock is necessary in a New function.
+ if (privileged) {
+ if (VIR_STRDUP(list->stateDir, SHMEM_STATE_DIR) < 0)
+ goto error;
+ } else {
+ char *rundir = NULL;
+
+ if (!(rundir = virGetUserRuntimeDirectory()))
+ goto error;
+
+ if (virAsprintf(&list->stateDir, "%s/shmem", rundir) < 0) {
+ VIR_FREE(rundir);
+ goto error;
+ }
+ VIR_FREE(rundir);
+ }
+ if (virFileMakePath(list->stateDir) < 0) {
+ virReportError(VIR_ERR_OPERATION_FAILED,
+ _("Failed to create state dir '%s'"),
+ list->stateDir);
+ goto error;
+ }
+ if (virShmObjectListLoadAllState(list) < 0)
+ goto error;
+
+ virObjectUnlock(list);
+ return list;
+
+ error:
+ virObjectUnlock(list);
+ virObjectUnref(list);
+ return NULL;
+}
+
+static int
+virShmOnceInit(void)
+{
+ if (!(virShmObjectListClass = virClassNew(virClassForObjectLockable(),
+ "virShmObjectList",
+ sizeof(virShmObjectList),
+ virShmObjectListDispose)))
+ return -1;
+
+ if (!(mainlist = virShmObjectListNew()))
+ return -1;
+
+ return 0;
+}
+
+VIR_ONCE_GLOBAL_INIT(virShm)
+
+virShmObjectListPtr
+virShmObjectListGetDefault(void)
+{
+ if (virShmInitialize() < 0)
+ return NULL;
+
+ return virObjectRef(mainlist);
+}
+
+int
+virShmSetUsedDomain(virShmObjectPtr shmobj,
+ const char *drvname,
+ const char *domname)
+{
+ char *tmpdomain = NULL;
+
+ if (shmobj->drvname) {
+ if (STRNEQ(drvname, shmobj->drvname)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("cannot use one shmem for different driver"));
+ goto error;
+ }
I don't I understand the limitation there. Imho, this could be just a warning.
+ } else {
+ if (VIR_STRDUP(shmobj->drvname, drvname) < 0)
+ goto error;
+ }
+
+ if (VIR_STRDUP(tmpdomain, domname) < 0)
+ goto error;
+
+ if (VIR_APPEND_ELEMENT(shmobj->domains, shmobj->ndomains, tmpdomain) < 0)
+ goto error;
+
+ return 0;
+
+ error:
+ VIR_FREE(tmpdomain);
+ return -1;
+}
+
+void
+virShmObjectFree(virShmObjectPtr shmobj)
+{
+ size_t i;
+
+ if (!shmobj)
+ return;
+
+ VIR_FREE(shmobj->name);
+ VIR_FREE(shmobj->path);
+ VIR_FREE(shmobj->drvname);
+ for (i = 0; i < shmobj->ndomains; i++)
+ VIR_FREE(shmobj->domains[i]);
+ VIR_FREE(shmobj->domains);
+ VIR_FREE(shmobj);
+}
+
+virShmObjectPtr
+virShmObjectNew(const char *name,
+ unsigned long long size,
+ const char *path,
+ int type,
+ bool othercreate,
+ const char *drvname,
+ const char *domname)
+{
+ virShmObjectPtr shmobj;
+
+ if (VIR_ALLOC(shmobj) < 0)
+ return NULL;
+
+ shmobj->size = size;
+ shmobj->type = type;
+
+ if (VIR_STRDUP(shmobj->name, name) < 0)
+ goto error;
+
+ if (path) {
+ if (VIR_STRDUP(shmobj->path, path) < 0)
+ goto error;
+ } else {
+ VIR_FREE(shmobj->path);
+ }
+ shmobj->othercreate = othercreate;
+
+ if (virShmSetUsedDomain(shmobj, drvname, domname) < 0)
+ goto error;
+
+ return shmobj;
+
+ error:
+ virShmObjectFree(shmobj);
+ return NULL;
+}
+
+static void
+virShmObjectListDispose(void *obj)
+{
+ virShmObjectListPtr list = obj;
+ size_t i;
+
+ for (i = 0; i < list->nshmobjs; i++)
+ virShmObjectFree(list->shmobjs[i]);
+
+ VIR_FREE(list->shmobjs);
+}
+
+int
+virShmObjectSaveState(virShmObjectPtr shmobj,
+ const char *stateDir)
+{
+ virBuffer buf = VIR_BUFFER_INITIALIZER;
+ int ret = -1;
+ size_t i;
+ char *xml = NULL;
+ char *stateFile = NULL;
+
+ virBufferAddLit(&buf, "<shmem>\n");
+ virBufferAdjustIndent(&buf, 2);
+
+ virBufferAsprintf(&buf, "<name>%s</name>\n",
shmobj->name);
+
+ virBufferAsprintf(&buf, "<type>%s</type>\n",
virShmObjectTypeToString(shmobj->type));
+
+ virBufferAsprintf(&buf, "<size>%llu</size>\n",
shmobj->size);
+ if (shmobj->path)
+ virBufferAsprintf(&buf, "<path>%s</path>\n",
shmobj->path);
+ if (shmobj->othercreate)
+ virBufferAddLit(&buf, "<othercreate/>\n");
+ virBufferAsprintf(&buf, "<driver>%s</driver>\n",
shmobj->drvname);
+ for (i = 0; i < shmobj->ndomains; i++)
+ virBufferAsprintf(&buf, "<domain name='%s'/>\n",
shmobj->domains[i]);
+
+ virBufferAdjustIndent(&buf, -2);
+ virBufferAddLit(&buf, "</shmem>\n");
+
+ if (virBufferCheckError(&buf) < 0)
+ goto cleanup;
+
+ if (!(xml = virBufferContentAndReset(&buf)))
+ goto cleanup;
+
+ if (!(stateFile = virFileBuildPath(stateDir, shmobj->name, ".xml")))
+ goto cleanup;
+
+ if (virXMLSaveFile(stateFile, NULL, NULL, xml) < 0)
+ goto cleanup;
+
+ ret = 0;
+
+ cleanup:
+ VIR_FREE(stateFile);
+ VIR_FREE(xml);
+ return ret;
+}
+
+int
+virShmRemoveUsedDomain(virShmObjectPtr shmobj,
+ const char *drvname,
+ const char *domname)
+{
+ size_t i;
+
+ if (shmobj->drvname) {
+ if (STRNEQ(drvname, shmobj->drvname)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("cannot use one shmem for different driver"));
+ return -1;
+ }
+ } else {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("cannot get shmem object driver name"));
+ return -1;
+ }
+
+ for (i = 0; i < shmobj->ndomains; i++) {
+ if (STREQ(shmobj->domains[i], domname)) {
+ VIR_FREE(shmobj->domains[i]);
+ VIR_DELETE_ELEMENT(shmobj->domains, i, shmobj->ndomains);
+ return 0;
+ }
+ }
+
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("cannot find domname in shmem object domain list"));
+ return -1;
+}
+
+int
+virShmObjectRemoveStateFile(virShmObjectListPtr list,
+ const char *name)
+{
+ char *stateFile = NULL;
+
+ if (!(stateFile = virFileBuildPath(list->stateDir, name, ".xml")))
+ return -1;
+ unlink(stateFile);
+ VIR_FREE(stateFile);
+ return 0;
+}
+
+int
+virShmObjectListDel(virShmObjectListPtr list,
+ virShmObjectPtr shmobj)
+{
+ size_t i;
+
+ for (i = 0; i < list->nshmobjs; i++) {
+ virShmObjectPtr tmp = list->shmobjs[i];
+
+ if (STREQ(tmp->name, shmobj->name)) {
+ VIR_DELETE_ELEMENT(list->shmobjs, i, list->nshmobjs);
+ return 0;
+ }
+ }
+ return -1;
+}
+
+virShmObjectPtr
+virShmObjectFindByName(virShmObjectListPtr list,
+ const char *name)
+{
+ size_t i;
+
+ for (i = 0; i < list->nshmobjs; i++) {
+ if (STREQ_NULLABLE(list->shmobjs[i]->name, name))
+ return list->shmobjs[i];
+ }
+
+ return NULL;
+}
+
+int
+virShmObjectListAdd(virShmObjectListPtr list,
+ virShmObjectPtr shmobj)
+{
+ if (virShmObjectFindByName(list, shmobj->name)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("A share memory object named '%s' already
exists"),
+ shmobj->name);
+ return -1;
+ }
+
+ return VIR_APPEND_ELEMENT(list->shmobjs, list->nshmobjs, shmobj);
+}
+
+#ifdef HAVE_SHM_OPEN
+int
+virShmOpen(const char *name,
+ unsigned long long size,
+ mode_t mode)
+{
+ int fd = -1;
+ struct stat sb;
+
+ if ((fd = shm_open(name, O_RDWR, mode)) < 0) {
+ virReportSystemError(errno,
+ _("Unable to open shared memory"
+ " objects '%s'"),
+ name);
+ return -1;
+ }
+
+ if (fstat(fd, &sb) < 0) {
+ virReportSystemError(errno,
+ _("cannot stat file '%s'"), name);
+ goto error;
+ }
+
+ if (sb.st_size < size) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("already exist shared memory object"
+ " size %ju is smaller than required size %llu"),
+ (uintmax_t)sb.st_size, size);
+ goto error;
+ }
+
+ return fd;
+
+ error:
+ VIR_FORCE_CLOSE(fd);
+ return -1;
+}
+
+int
+virShmCreate(const char *name,
+ unsigned long long size,
+ bool outerr,
+ bool *othercreate,
+ mode_t mode)
+{
+ int fd = -1;
+
+ fd = shm_open(name, O_RDWR|O_CREAT|O_EXCL, mode);
+
+ if (fd > 0) {
+ if (ftruncate(fd, size) != 0) {
+ virReportSystemError(errno,
+ _("Unable to truncate"
+ " file descriptor to %llu"),
+ size);
+ ignore_value(shm_unlink(name));
+ VIR_FORCE_CLOSE(fd);
+ return -1;
+ }
+ } else if (errno == EEXIST) {
+ if (outerr) {
+ virReportSystemError(errno,
+ _("shared memory objects"
+ " '%s' is already exist"),
+ name);
+ return -1;
+ }
+
+ VIR_WARN("shared memory objects '%s' is already exist",
name);
+ *othercreate = true;
+
+ return virShmOpen(name, size, mode);
+ } else {
+ virReportSystemError(errno,
+ _("Unable to create shared memory"
+ " objects '%s'"),
+ name);
+ return -1;
+ }
+
+ return fd;
+}
+
+int
+virShmUnlink(const char *name)
+{
+ int ret;
+
+ if ((ret = shm_unlink(name)) < 0) {
+ virReportSystemError(errno,
+ _("Unable to delete shared memory"
+ " objects '%s'"),
+ name);
+ }
+ return ret;
+}
+
+# if defined(__linux__)
+# define SHM_DEFAULT_PATH "/dev/shm"
+
+int
+virShmBuildPath(const char *name, char **path)
+{
+
+ if (virAsprintf(path, "%s/%s", SHM_DEFAULT_PATH, name) < 0)
+ return -1;
+
+ if (!virFileExists(*path)) {
+ virReportSystemError(errno,
+ _("could not access %s"),
+ *path);
+ VIR_FREE(*path);
+ return -1;
+ }
+ return 0;
+}
+
+# else
+
+int
+virShmBuildPath(const char *name ATTRIBUTE_UNUSED,
+ char **path ATTRIBUTE_UNUSED)
+{
+ virReportSystemError(ENOSYS, "%s",
+ _("Cannot get share memory object path on this
platform"));
+ return -2;
+}
+
+# endif
+#endif /* HAVE_SHM_OPEN */
diff --git a/src/util/virshm.h b/src/util/virshm.h
new file mode 100644
index 0000000..945a541
--- /dev/null
+++ b/src/util/virshm.h
@@ -0,0 +1,104 @@
+/*
+ * virshm.h: helper API for POSIX share memory
+ *
+ * Copyright (C) 2015 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see
+ * <http://www.gnu.org/licenses/>;.
+ *
+ */
+
+#ifndef __VIR_SHM_H__
+# define __VIR_SHM_H__
+
+# include "internal.h"
+# include "virutil.h"
+# include "virobject.h"
+
+typedef enum {
+ VIR_SHM_TYPE_SHM = 0,
+ VIR_SHM_TYPE_SERVER,
+
+ VIR_SHM_TYPE_LAST
+} virShmObjectType;
+
+typedef struct _virShmObject virShmObject;
+typedef virShmObject *virShmObjectPtr;
+struct _virShmObject {
+ char *name; /* shmem object name */
+ int type; /* shmem object type */
+ unsigned long long size; /* size of shmem object */
+ char *path; /* shmem path */
+ bool othercreate; /* a bool parameter record if the shm is created by
libvirt */
+
+ char *drvname; /* which driver */
+ char **domains; /* domain(s) using this shm */
+ size_t ndomains; /* number of useds */
+};
+
+VIR_ENUM_DECL(virShmObject);
+
+typedef struct _virShmObjectList virShmObjectList;
+typedef virShmObjectList *virShmObjectListPtr;
+struct _virShmObjectList {
+ virObjectLockable parent;
+ char *stateDir;
+ size_t nshmobjs;
+ virShmObjectPtr *shmobjs;
+};
+
+virShmObjectListPtr virShmObjectListGetDefault(void);
+
+int virShmSetUsedDomain(virShmObjectPtr shmobj, const char *drvname, const char
*domname)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
+int virShmRemoveUsedDomain(virShmObjectPtr shmobj, const char *drvname, const char
*domname)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
+void virShmObjectFree(virShmObjectPtr shmobj);
+virShmObjectPtr virShmObjectNew(const char *name, unsigned long long size,
+ const char *path, int type, bool othercreate,
+ const char *drvname, const char *domname)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(6)
+ ATTRIBUTE_NONNULL(7);
+int virShmObjectSaveState(virShmObjectPtr shmobj, const char *stateDir)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+int virShmObjectRemoveStateFile(virShmObjectListPtr list, const char *name)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+int virShmObjectListAdd(virShmObjectListPtr list, virShmObjectPtr shmobj)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+int virShmObjectListDel(virShmObjectListPtr list, virShmObjectPtr shmobj)
+ ATTRIBUTE_NONNULL(1);
+virShmObjectPtr virShmObjectFindByName(virShmObjectListPtr list,
+ const char *name)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+
+# ifdef HAVE_SHM_OPEN
+int virShmCreate(const char *name, unsigned long long size,
+ bool outerr, bool *othercreate, mode_t mode)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(4);
+int virShmOpen(const char *name,
+ unsigned long long size,
+ mode_t mode)
+ ATTRIBUTE_NONNULL(1);
+int virShmUnlink(const char *name)
+ ATTRIBUTE_NONNULL(1);
+int virShmBuildPath(const char *name, char **path)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+# else /* HAVE_SHM_OPEN */
+# define virShmCreate(name, size, outerr, othercreate, mode) -2
+# define virShmOpen(name, size, mode) -2
+# define virShmUnlink(name) -2
+# define virShmBuildPath(name, path) -2
+
+# endif /* HAVE_SHM_OPEN */
+#endif /* __VIR_SHM_H__ */
--
1.8.3.1
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
--
Marc-André Lureau
9 a.m.
New subject: [libvirt] [PATCH 3/4] util: introduce new helpers to manage shmem device
Hi Marc-André
On 07/27/2015 11:43 PM, Marc-André Lureau wrote:
Hi
On Thu, Jul 23, 2015 at 12:13 PM, Luyao Huang <lhuang(a)redhat.com> wrote:
> Signed-off-by: Luyao Huang <lhuang(a)redhat.com > ---
> configure.ac | 10 +
> po/POTFILES.in | 3 +-
> src/Makefile.am | 5 +-
> src/libvirt_private.syms | 16 ++
> src/util/virshm.c | 623 +++++++++++++++++++++++++++++++++++++++++++++++
> src/util/virshm.h | 104 ++++++++
> 6 files changed, 759 insertions(+), 2 deletions(-)
> create mode 100644 src/util/virshm.c
> create mode 100644 src/util/virshm.h
(would be nicer to have some tests)
After applying this patch, make check fails with
Symbol block at ./libvirt_private.syms:2081: symbols not sorted
virShmObjectFree
virShmObjectNew
virShmObjectListAdd
virShmObjectListDel
virShmObjectFindByName
virShmObjectListGetDefault
Correct ordering
virShmObjectFindByName
virShmObjectFree
virShmObjectListAdd
virShmObjectListDel
virShmObjectListGetDefault
virShmObjectNew
Makefile:10195: recipe for target 'check-symsorting' failed
Oh, my fault, i forgot this, thanks for pointing out.
> diff --git a/configure.ac b/configure.ac
> index a7f38e8..940eb66 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1176,6 +1176,16 @@ if test "$with_linux" = "yes"; then
> ]])
> fi
> +dnl
> +dnl check for POSIX share memory functions
> +dnl
> +LIBRT_LIBS=""
> +AC_CHECK_LIB([rt],[shm_open],[LIBRT_LIBS="-lrt"])
> +old_libs="$LIBS"
> +LIBS="$old_libs $LIBRT_LIBS"
> +AC_CHECK_FUNCS([shm_open])
> +LIBS="$old_libs"
> +AC_SUBST([LIBRT_LIBS])
> dnl Need to test if pkg-config exists
> PKG_PROG_PKG_CONFIG
> diff --git a/po/POTFILES.in b/po/POTFILES.in
> index a75f5ae..7687a82 100644
> --- a/po/POTFILES.in
> +++ b/po/POTFILES.in
> @@ -215,8 +215,9 @@ src/util/virpolkit.c
> src/util/virportallocator.c
> src/util/virprocess.c
> src/util/virrandom.c
> -src/util/virsexpr.c
> src/util/virscsi.c
> +src/util/virsexpr.c
> +src/util/virshm.c
> src/util/virsocketaddr.c
> src/util/virstats.c
> src/util/virstorageencryption.c
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 7338ab9..048a096 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -152,6 +152,7 @@ UTIL_SOURCES =
\
> util/virscsi.c util/virscsi.h \
> util/virseclabel.c util/virseclabel.h \
> util/virsexpr.c util/virsexpr.h \
> + util/virshm.c util/virshm.h \
> util/virsocketaddr.h util/virsocketaddr.c \
> util/virstats.c util/virstats.h \
> util/virstorageencryption.c util/virstorageencryption.h \
> @@ -1048,7 +1049,7 @@ libvirt_util_la_LIBADD = $(CAPNG_LIBS) $(YAJL_LIBS)
$(LIBNL_LIBS) \
> $(THREAD_LIBS) $(AUDIT_LIBS) $(DEVMAPPER_LIBS) \
> $(LIB_CLOCK_GETTIME) $(DBUS_LIBS) $(MSCOM_LIBS) $(LIBXML_LIBS) \
> $(SECDRIVER_LIBS) $(NUMACTL_LIBS) $(SYSTEMD_DAEMON_LIBS) \
> - $(POLKIT_LIBS)
> + $(POLKIT_LIBS) $(LIBRT_LIBS)
> noinst_LTLIBRARIES +=
libvirt_conf.la
> @@ -1284,6 +1285,7 @@ libvirt_driver_qemu_impl_la_LIBADD = $(CAPNG_LIBS) \
> $(GNUTLS_LIBS) \
> $(LIBNL_LIBS) \
> $(LIBXML_LIBS) \
> + $(LIBRT_LIBS) \
> $(NULL)
> libvirt_driver_qemu_impl_la_SOURCES = $(QEMU_DRIVER_SOURCES)
> @@ -2264,6 +2266,7 @@ libvirt_setuid_rpc_client_la_LDFLAGS
= \
> $(AM_LDFLAGS) \
> $(LIBXML_LIBS) \
> $(SECDRIVER_LIBS) \
> + $(LIBRT_LIBS) \
> $(NULL)
> libvirt_setuid_rpc_client_la_CFLAGS = \
> -DLIBVIRT_SETUID_RPC_CLIENT \
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index af73177..977fd34 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -2078,6 +2078,22 @@ sexpr_u64;
> string2sexpr;
> +# util/virshm.h
> +virShmBuildPath;
> +virShmCreate;
> +virShmObjectFree;
> +virShmObjectNew;
> +virShmObjectListAdd;
> +virShmObjectListDel;
> +virShmObjectFindByName;
> +virShmObjectListGetDefault;
> +virShmObjectRemoveStateFile;
> +virShmObjectSaveState;
> +virShmOpen;
> +virShmRemoveUsedDomain;
> +virShmSetUsedDomain;
> +virShmUnlink;
So these lines should be sorted
Yes, i will fix them in next version
> +
> # util/virsocketaddr.h
> virSocketAddrBroadcast;
> virSocketAddrBroadcastByPrefix;
> diff --git a/src/util/virshm.c b/src/util/virshm.c
> new file mode 100644
> index 0000000..7ab39be
> --- /dev/null
> +++ b/src/util/virshm.c
> @@ -0,0 +1,623 @@
> +/*
> + * virshm.c: helper API for POSIX share memory
> + *
> + * Copyright (C) 2015 Red Hat, Inc.
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library. If not, see
> + * <http://www.gnu.org/licenses/>;.
> + *
> + */
> +
> +#include <config.h > +
> +#include <fcntl.h > +#include <sys/stat.h > +#include <sys/types.h > +
> +#ifdef HAVE_SHM_OPEN
> +# include <sys/mman.h > +#endif
> +
> +#include "virshm.h"
> +#include "virxml.h"
> +#include "virbuffer.h"
> +#include "virerror.h"
> +#include "virstring.h"
> +#include "virlog.h"
> +#include "virutil.h"
> +#include "viralloc.h"
> +#include "virfile.h"
> +#include "configmake.h"
> +
> +#define VIR_FROM_THIS VIR_FROM_NONE
> +
> +VIR_LOG_INIT("util.shm");
> +
> +#define SHMEM_STATE_DIR LOCALSTATEDIR "/run/libvirt/shmem"
> +
> +VIR_ENUM_IMPL(virShmObject, VIR_SHM_TYPE_LAST,
> + "shm",
> + "server");
> +
> +static virClassPtr virShmObjectListClass;
> +
> +static virShmObjectListPtr mainlist; /* global shm object list */
> +
> +static void virShmObjectListDispose(void *obj);
> +
> +static int
> +virShmObjectListLoadState(virShmObjectPtr *shmobj,
> + const char *stateDir,
> + const char *name)
> +{
> + char *stateFile = NULL;
> + xmlXPathContextPtr ctxt = NULL;
> + xmlDocPtr xml = NULL;
> + virShmObjectPtr tmpshm;
> + xmlNodePtr *usagenode = NULL;
> + xmlNodePtr save = NULL;
save could be moved in the using scope
Okay
> + int ret = -1;
> + char *drivername = NULL;
> + char *shmtype = NULL;
> + int nusages;
> +
> + if (VIR_ALLOC(tmpshm) < 0)
> + return -1;
> +
> + if (!(stateFile = virFileBuildPath(stateDir, name, ".xml")))
> + goto error;
> +
> + if (!(xml = virXMLParseFileCtxt(stateFile, &ctxt)))
> + goto error;
> +
> + tmpshm->name = virXPathString("string(./name)", ctxt);
> + if (!tmpshm->name) {
> + virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("shmem missing name attribute"));
> + goto error;
> + }
> +
> + shmtype = virXPathString("string(./type)", ctxt);
> + if (!shmtype) {
> + virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("shmem missing type attribute"));
> + goto error;
> + }
> + if ((tmpshm->type = virShmObjectTypeFromString(shmtype)) < 0) {
> + virReportError(VIR_ERR_XML_ERROR,
> + _("invalid shmem object type %s"), shmtype);
You leak shmtype here and in other goto error
> + goto error;
> + }
> +
> + if (virXPathULongLong("string(./size)", ctxt, &tmpshm->size)
< 0) {
> + virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("shmem missing or have invalid size
attribute"));
> + goto error;
> + }
> +
> + tmpshm->path = virXPathString("string(./path)", ctxt);
> + if (virXPathBoolean("boolean(./othercreate)", ctxt))
> + tmpshm->othercreate = true;
> +
> + if (!(drivername = virXPathString("string(./@driver)", ctxt))) {
> + virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("shmem usage element missing driver
attribute"));
> + goto error;
> + }
> + nusages = virXPathNodeSet("./domain", ctxt, &usagenode);
> + if (nusages < 0)
> + goto error;
> +
> + if (nusages > 0) {
> + size_t i;
> +
> + for (i = 0; i < nusages; i++) {
> + char *domainname;
> +
> + save = ctxt->node;
> + ctxt->node = usagenode[i];
> +
> + if (!(domainname = virXPathString("string(./@name)", ctxt)))
{
> + virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("shmem domain element missing name
attribute"));
> + goto error;
> + }
> +
> + if (virShmSetUsedDomain(tmpshm, drivername, domainname) < 0) {
> + VIR_FREE(domainname);
> + goto error;
> + }
> + VIR_FREE(domainname);
> + ctxt->node = save;
> + }
> + }
> + *shmobj = tmpshm;
> + ret = 0;
> + cleanup:
> + VIR_FREE(stateFile);
> + VIR_FREE(drivername);
> + VIR_FREE(shmtype);
> + xmlFreeDoc(xml);
> + xmlXPathFreeContext(ctxt);
> + return ret;
> +
> + error:
> + virShmObjectFree(tmpshm);
> + goto cleanup;
> +}
> +
> +static int
> +virShmObjectListLoadAllState(virShmObjectListPtr list)
> +{
> + DIR *dir;
> + struct dirent *entry;
> +
> + if (!(dir = opendir(list->stateDir))) {
> + if (errno == ENOENT)
> + return 0;
> + virReportSystemError(errno, _("Failed to open dir '%s'"),
list->stateDir);
> + return -1;
> + }
> +
> + while (virDirRead(dir, &entry, list->stateDir) > 0) {
> + virShmObjectPtr shmobj;
> +
> + if (entry->d_name[0] == '.' ||
> + !virFileStripSuffix(entry->d_name, ".xml"))
> + continue;
> + if (virShmObjectListLoadState(&shmobj, list->stateDir,
entry->d_name) < 0)
> + continue;
> + if (virShmObjectListAdd(list, shmobj) < 0)
> + continue;
> + }
> + closedir(dir);
> + return 0;
> +}
> +
> +static virShmObjectListPtr
> +virShmObjectListNew(void)
> +{
> + virShmObjectListPtr list;
> + bool privileged = geteuid() == 0;
> +
> + if (!(list = virObjectLockableNew(virShmObjectListClass)))
> + return NULL;
> +
> + virObjectLock(list);
I am not sure the lock is necessary in a New function.
Hmm...indeed, maybe we could remove the lock here.
> + if (privileged) {
> + if (VIR_STRDUP(list->stateDir, SHMEM_STATE_DIR) < 0)
> + goto error;
> + } else {
> + char *rundir = NULL;
> +
> + if (!(rundir = virGetUserRuntimeDirectory()))
> + goto error;
> +
> + if (virAsprintf(&list->stateDir, "%s/shmem", rundir) <
0) {
> + VIR_FREE(rundir);
> + goto error;
> + }
> + VIR_FREE(rundir);
> + }
> + if (virFileMakePath(list->stateDir) < 0) {
> + virReportError(VIR_ERR_OPERATION_FAILED,
> + _("Failed to create state dir '%s'"),
> + list->stateDir);
> + goto error;
> + }
> + if (virShmObjectListLoadAllState(list) < 0)
> + goto error;
> +
> + virObjectUnlock(list);
> + return list;
> +
> + error:
> + virObjectUnlock(list);
> + virObjectUnref(list);
> + return NULL;
> +}
> +
> +static int
> +virShmOnceInit(void)
> +{
> + if (!(virShmObjectListClass = virClassNew(virClassForObjectLockable(),
> + "virShmObjectList",
> + sizeof(virShmObjectList),
> + virShmObjectListDispose)))
> + return -1;
> +
> + if (!(mainlist = virShmObjectListNew()))
> + return -1;
> +
> + return 0;
> +}
> +
> +VIR_ONCE_GLOBAL_INIT(virShm)
> +
> +virShmObjectListPtr
> +virShmObjectListGetDefault(void)
> +{
> + if (virShmInitialize() < 0)
> + return NULL;
> +
> + return virObjectRef(mainlist);
> +}
> +
> +int
> +virShmSetUsedDomain(virShmObjectPtr shmobj,
> + const char *drvname,
> + const char *domname)
> +{
> + char *tmpdomain = NULL;
> +
> + if (shmobj->drvname) {
> + if (STRNEQ(drvname, shmobj->drvname)) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("cannot use one shmem for different
driver"));
> + goto error;
> + }
I don't I understand the limitation there. Imho, this could be just a warning.
In my opinion, shmem device could be used in different driver (although
only qemu use it right now). The shmem device set up part will be
different in different driver, it will cause some strange issue during
the set up and clean up part.
Thanks a lot for your review and help.
Luyao
5:12 a.m.
New subject: [libvirt] [PATCH 3/4] util: introduce new helpers to manage shmem device
On Thu, Jul 23, 2015 at 06:13:48PM +0800, Luyao Huang wrote:
Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
---
configure.ac | 10 +
po/POTFILES.in | 3 +-
src/Makefile.am | 5 +-
src/libvirt_private.syms | 16 ++
src/util/virshm.c | 623 +++++++++++++++++++++++++++++++++++++++++++++++
src/util/virshm.h | 104 ++++++++
6 files changed, 759 insertions(+), 2 deletions(-)
create mode 100644 src/util/virshm.c
create mode 100644 src/util/virshm.h
diff --git a/src/Makefile.am b/src/Makefile.am
index 7338ab9..048a096 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -152,6 +152,7 @@ UTIL_SOURCES = \
noinst_LTLIBRARIES += libvirt_conf.la
@@ -1284,6 +1285,7 @@ libvirt_driver_qemu_impl_la_LIBADD = $(CAPNG_LIBS) \
$(GNUTLS_LIBS) \
$(LIBNL_LIBS) \
$(LIBXML_LIBS) \
+ $(LIBRT_LIBS) \
$(NULL)
libvirt_driver_qemu_impl_la_SOURCES = $(QEMU_DRIVER_SOURCES)
@@ -2264,6 +2266,7 @@ libvirt_setuid_rpc_client_la_LDFLAGS = \
$(AM_LDFLAGS) \
$(LIBXML_LIBS) \
$(SECDRIVER_LIBS) \
+ $(LIBRT_LIBS) \
$(NULL)
libvirt_setuid_rpc_client_la_CFLAGS = \
-DLIBVIRT_SETUID_RPC_CLIENT \
Indentation is messed up for these
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index af73177..977fd34 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2078,6 +2078,22 @@ sexpr_u64;
string2sexpr;
+# util/virshm.h
+virShmBuildPath;
+virShmCreate;
+virShmObjectFree;
+virShmObjectNew;
+virShmObjectListAdd;
+virShmObjectListDel;
+virShmObjectFindByName;
+virShmObjectListGetDefault;
+virShmObjectRemoveStateFile;
+virShmObjectSaveState;
+virShmOpen;
+virShmRemoveUsedDomain;
+virShmSetUsedDomain;
+virShmUnlink;
+
Did you run 'make check' because they should fail on the
sort ordering test.
diff --git a/src/util/virshm.c b/src/util/virshm.c
new file mode 100644
index 0000000..7ab39be
--- /dev/null
+++ b/src/util/virshm.c
+#include <config.h>
+
+#include <fcntl.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+
+#ifdef HAVE_SHM_OPEN
+# include <sys/mman.h>
+#endif
+
+#include "virshm.h"
+#include "virxml.h"
+#include "virbuffer.h"
+#include "virerror.h"
+#include "virstring.h"
+#include "virlog.h"
+#include "virutil.h"
+#include "viralloc.h"
+#include "virfile.h"
+#include "configmake.h"
+
+#define VIR_FROM_THIS VIR_FROM_NONE
+
+VIR_LOG_INIT("util.shm");
+
+#define SHMEM_STATE_DIR LOCALSTATEDIR "/run/libvirt/shmem"
+
+VIR_ENUM_IMPL(virShmObject, VIR_SHM_TYPE_LAST,
+ "shm",
+ "server");
+
+static virClassPtr virShmObjectListClass;
+
+static virShmObjectListPtr mainlist; /* global shm object list */
+
+static void virShmObjectListDispose(void *obj);
+int
+virShmSetUsedDomain(virShmObjectPtr shmobj,
+ const char *drvname,
+ const char *domname)
+{
+ char *tmpdomain = NULL;
+
+ if (shmobj->drvname) {
+ if (STRNEQ(drvname, shmobj->drvname)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("cannot use one shmem for different driver"));
+ goto error;
+ }
+ } else {
+ if (VIR_STRDUP(shmobj->drvname, drvname) < 0)
+ goto error;
+ }
+
+ if (VIR_STRDUP(tmpdomain, domname) < 0)
+ goto error;
+
+ if (VIR_APPEND_ELEMENT(shmobj->domains, shmobj->ndomains, tmpdomain) < 0)
+ goto error;
+
+ return 0;
+
+ error:
+ VIR_FREE(tmpdomain);
In this error codepath you have possibly already set
shmobj->drvname, so you need to be able to undo that.
+ return -1;
+}
+#ifdef HAVE_SHM_OPEN
+int
+virShmOpen(const char *name,
+ unsigned long long size,
+ mode_t mode)
+{
+ int fd = -1;
+ struct stat sb;
+
+ if ((fd = shm_open(name, O_RDWR, mode)) < 0) {
+ virReportSystemError(errno,
+ _("Unable to open shared memory"
+ " objects '%s'"),
+ name);
+ return -1;
+ }
+
+ if (fstat(fd, &sb) < 0) {
+ virReportSystemError(errno,
+ _("cannot stat file '%s'"), name);
+ goto error;
+ }
+
+ if (sb.st_size < size) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("already exist shared memory object"
+ " size %ju is smaller than required size %llu"),
+ (uintmax_t)sb.st_size, size);
+ goto error;
I think this needs to be != instead of <
Consider Domain A with shmsize=1024 and Domain B with shmsize=2048
The check will raise an error if domain A is started before domain B
preventing both A & B running at same time. If B is started before
A though, it will happily let both run.
+int
+virShmCreate(const char *name,
+ unsigned long long size,
+ bool outerr,
+ bool *othercreate,
+ mode_t mode)
+{
+ int fd = -1;
+
+ fd = shm_open(name, O_RDWR|O_CREAT|O_EXCL, mode);
+
+ if (fd > 0) {
+ if (ftruncate(fd, size) != 0) {
+ virReportSystemError(errno,
+ _("Unable to truncate"
+ " file descriptor to %llu"),
+ size);
+ ignore_value(shm_unlink(name));
+ VIR_FORCE_CLOSE(fd);
+ return -1;
+ }
+ } else if (errno == EEXIST) {
+ if (outerr) {
+ virReportSystemError(errno,
+ _("shared memory objects"
+ " '%s' is already exist"),
+ name);
+ return -1;
+ }
+
+ VIR_WARN("shared memory objects '%s' is already exist",
name);
+ *othercreate = true;
+
You're leaking 'fd' in this case
+ return virShmOpen(name, size, mode);
+ } else {
+ virReportSystemError(errno,
+ _("Unable to create shared memory"
+ " objects '%s'"),
+ name);
+ return -1;
+ }
+
+ return fd;
+}
+
+int
+virShmUnlink(const char *name)
+{
+ int ret;
+
+ if ((ret = shm_unlink(name)) < 0) {
+ virReportSystemError(errno,
+ _("Unable to delete shared memory"
+ " objects '%s'"),
+ name);
+ }
+ return ret;
+}
+
+# if defined(__linux__)
+# define SHM_DEFAULT_PATH "/dev/shm"
+
+int
+virShmBuildPath(const char *name, char **path)
+{
+
+ if (virAsprintf(path, "%s/%s", SHM_DEFAULT_PATH, name) < 0)
+ return -1;
+
+ if (!virFileExists(*path)) {
+ virReportSystemError(errno,
+ _("could not access %s"),
+ *path);
+ VIR_FREE(*path);
+ return -1;
+ }
+ return 0;
+}
+
+# else
+
+int
+virShmBuildPath(const char *name ATTRIBUTE_UNUSED,
+ char **path ATTRIBUTE_UNUSED)
+{
+ virReportSystemError(ENOSYS, "%s",
+ _("Cannot get share memory object path on this
platform"));
+ return -2;
+}
Why -2 instead of -1 ?
diff --git a/src/util/virshm.h b/src/util/virshm.h
new file mode 100644
index 0000000..945a541
--- /dev/null
+++ b/src/util/virshm.h
+# include "internal.h"
+# include "virutil.h"
+# include "virobject.h"
+
+typedef enum {
+ VIR_SHM_TYPE_SHM = 0,
+ VIR_SHM_TYPE_SERVER,
+
+ VIR_SHM_TYPE_LAST
+} virShmObjectType;
I'm unclear why we need the different type constants
here ?
+
+typedef struct _virShmObject virShmObject;
+typedef virShmObject *virShmObjectPtr;
+struct _virShmObject {
+ char *name; /* shmem object name */
+ int type; /* shmem object type */
+ unsigned long long size; /* size of shmem object */
+ char *path; /* shmem path */
+ bool othercreate; /* a bool parameter record if the shm is created by
libvirt */
+
+ char *drvname; /* which driver */
+ char **domains; /* domain(s) using this shm */
+ size_t ndomains; /* number of useds */
+};
+
+VIR_ENUM_DECL(virShmObject);
+
+typedef struct _virShmObjectList virShmObjectList;
+typedef virShmObjectList *virShmObjectListPtr;
+struct _virShmObjectList {
+ virObjectLockable parent;
+ char *stateDir;
+ size_t nshmobjs;
+ virShmObjectPtr *shmobjs;
+};
I think of the struct definitions should be private. If
callers need access to the contents we can provide APIs
+# ifdef HAVE_SHM_OPEN
+int virShmCreate(const char *name, unsigned long long size,
+ bool outerr, bool *othercreate, mode_t mode)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(4);
+int virShmOpen(const char *name,
+ unsigned long long size,
+ mode_t mode)
+ ATTRIBUTE_NONNULL(1);
+int virShmUnlink(const char *name)
+ ATTRIBUTE_NONNULL(1);
+int virShmBuildPath(const char *name, char **path)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+# else /* HAVE_SHM_OPEN */
+# define virShmCreate(name, size, outerr, othercreate, mode) -2
+# define virShmOpen(name, size, mode) -2
+# define virShmUnlink(name) -2
+# define virShmBuildPath(name, path) -2
+
+# endif /* HAVE_SHM_OPEN */
You can't just #define away the APIs - this will lead to undefined
symbols being referenced in the linker script which breaks on
Win32. You need to provide stub implementations. Also you need to
use virReportError() any time you return an error status, as you
already did for virShmBuildPath.
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 :|
9 a.m.
New subject: [libvirt] [PATCH 3/4] util: introduce new helpers to manage shmem device
On 07/30/2015 06:12 PM, Daniel P. Berrange wrote:
On Thu, Jul 23, 2015 at 06:13:48PM +0800, Luyao Huang wrote:
> Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
> ---
> configure.ac | 10 +
> po/POTFILES.in | 3 +-
> src/Makefile.am | 5 +-
> src/libvirt_private.syms | 16 ++
> src/util/virshm.c | 623 +++++++++++++++++++++++++++++++++++++++++++++++
> src/util/virshm.h | 104 ++++++++
> 6 files changed, 759 insertions(+), 2 deletions(-)
> create mode 100644 src/util/virshm.c
> create mode 100644 src/util/virshm.h
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 7338ab9..048a096 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -152,6 +152,7 @@ UTIL_SOURCES = \
> noinst_LTLIBRARIES += libvirt_conf.la
> @@ -1284,6 +1285,7 @@ libvirt_driver_qemu_impl_la_LIBADD = $(CAPNG_LIBS) \
> $(GNUTLS_LIBS) \
> $(LIBNL_LIBS) \
> $(LIBXML_LIBS) \
> + $(LIBRT_LIBS) \
> $(NULL)
> libvirt_driver_qemu_impl_la_SOURCES = $(QEMU_DRIVER_SOURCES)
>
> @@ -2264,6 +2266,7 @@ libvirt_setuid_rpc_client_la_LDFLAGS = \
> $(AM_LDFLAGS) \
> $(LIBXML_LIBS) \
> $(SECDRIVER_LIBS) \
> + $(LIBRT_LIBS) \
> $(NULL)
> libvirt_setuid_rpc_client_la_CFLAGS = \
> -DLIBVIRT_SETUID_RPC_CLIENT \
Indentation is messed up for these
Okay, got it
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index af73177..977fd34 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -2078,6 +2078,22 @@ sexpr_u64;
> string2sexpr;
>
>
> +# util/virshm.h
> +virShmBuildPath;
> +virShmCreate;
> +virShmObjectFree;
> +virShmObjectNew;
> +virShmObjectListAdd;
> +virShmObjectListDel;
> +virShmObjectFindByName;
> +virShmObjectListGetDefault;
> +virShmObjectRemoveStateFile;
> +virShmObjectSaveState;
> +virShmOpen;
> +virShmRemoveUsedDomain;
> +virShmSetUsedDomain;
> +virShmUnlink;
> +
Did you run 'make check' because they should fail on the
sort ordering test.
Sorry, my fault, i forgot it, i will fix them in next version
> diff --git a/src/util/virshm.c b/src/util/virshm.c
> new file mode 100644
> index 0000000..7ab39be
> --- /dev/null
> +++ b/src/util/virshm.c
> +#include <config.h>
> +
> +#include <fcntl.h>
> +#include <sys/stat.h>
> +#include <sys/types.h>
> +
> +#ifdef HAVE_SHM_OPEN
> +# include <sys/mman.h>
> +#endif
> +
> +#include "virshm.h"
> +#include "virxml.h"
> +#include "virbuffer.h"
> +#include "virerror.h"
> +#include "virstring.h"
> +#include "virlog.h"
> +#include "virutil.h"
> +#include "viralloc.h"
> +#include "virfile.h"
> +#include "configmake.h"
> +
> +#define VIR_FROM_THIS VIR_FROM_NONE
> +
> +VIR_LOG_INIT("util.shm");
> +
> +#define SHMEM_STATE_DIR LOCALSTATEDIR "/run/libvirt/shmem"
> +
> +VIR_ENUM_IMPL(virShmObject, VIR_SHM_TYPE_LAST,
> + "shm",
> + "server");
> +
> +static virClassPtr virShmObjectListClass;
> +
> +static virShmObjectListPtr mainlist; /* global shm object list */
> +
> +static void virShmObjectListDispose(void *obj);
> +int
> +virShmSetUsedDomain(virShmObjectPtr shmobj,
> + const char *drvname,
> + const char *domname)
> +{
> + char *tmpdomain = NULL;
> +
> + if (shmobj->drvname) {
> + if (STRNEQ(drvname, shmobj->drvname)) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("cannot use one shmem for different
driver"));
> + goto error;
> + }
> + } else {
> + if (VIR_STRDUP(shmobj->drvname, drvname) < 0)
> + goto error;
> + }
> +
> + if (VIR_STRDUP(tmpdomain, domname) < 0)
> + goto error;
> +
> + if (VIR_APPEND_ELEMENT(shmobj->domains, shmobj->ndomains, tmpdomain) <
0)
> + goto error;
> +
> + return 0;
> +
> + error:
> + VIR_FREE(tmpdomain);
In this error codepath you have possibly already set
shmobj->drvname, so you need to be able to undo that.
oh, good catch, i will fix this issue in next version.
> + return -1;
> +}
> +#ifdef HAVE_SHM_OPEN
> +int
> +virShmOpen(const char *name,
> + unsigned long long size,
> + mode_t mode)
> +{
> + int fd = -1;
> + struct stat sb;
> +
> + if ((fd = shm_open(name, O_RDWR, mode)) < 0) {
> + virReportSystemError(errno,
> + _("Unable to open shared memory"
> + " objects '%s'"),
> + name);
> + return -1;
> + }
> +
> + if (fstat(fd, &sb) < 0) {
> + virReportSystemError(errno,
> + _("cannot stat file '%s'"), name);
> + goto error;
> + }
> +
> + if (sb.st_size < size) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("already exist shared memory object"
> + " size %ju is smaller than required size %llu"),
> + (uintmax_t)sb.st_size, size);
> + goto error;
I think this needs to be != instead of <
Consider Domain A with shmsize=1024 and Domain B with shmsize=2048
The check will raise an error if domain A is started before domain B
preventing both A & B running at same time. If B is started before
A though, it will happily let both run.
Indeed, i will modify the code to forbid use a shm if the size is not
equal,
> +int
> +virShmCreate(const char *name,
> + unsigned long long size,
> + bool outerr,
> + bool *othercreate,
> + mode_t mode)
> +{
> + int fd = -1;
> +
> + fd = shm_open(name, O_RDWR|O_CREAT|O_EXCL, mode);
> +
> + if (fd > 0) {
> + if (ftruncate(fd, size) != 0) {
> + virReportSystemError(errno,
> + _("Unable to truncate"
> + " file descriptor to %llu"),
> + size);
> + ignore_value(shm_unlink(name));
> + VIR_FORCE_CLOSE(fd);
> + return -1;
> + }
> + } else if (errno == EEXIST) {
> + if (outerr) {
> + virReportSystemError(errno,
> + _("shared memory objects"
> + " '%s' is already exist"),
> + name);
> + return -1;
> + }
> +
> + VIR_WARN("shared memory objects '%s' is already exist",
name);
> + *othercreate = true;
> +
You're leaking 'fd' in this case
Hmm...i couldn't find the leaking place, i guess your mean i will
leaking 'fd' when "errno == EEXIST", but at that time the fd <= 0.
> + return virShmOpen(name, size, mode);
> + } else {
> + virReportSystemError(errno,
> + _("Unable to create shared memory"
> + " objects '%s'"),
> + name);
> + return -1;
> + }
> +
> + return fd;
> +}
> +
> +int
> +virShmUnlink(const char *name)
> +{
> + int ret;
> +
> + if ((ret = shm_unlink(name)) < 0) {
> + virReportSystemError(errno,
> + _("Unable to delete shared memory"
> + " objects '%s'"),
> + name);
> + }
> + return ret;
> +}
> +
> +# if defined(__linux__)
> +# define SHM_DEFAULT_PATH "/dev/shm"
> +
> +int
> +virShmBuildPath(const char *name, char **path)
> +{
> +
> + if (virAsprintf(path, "%s/%s", SHM_DEFAULT_PATH, name) < 0)
> + return -1;
> +
> + if (!virFileExists(*path)) {
> + virReportSystemError(errno,
> + _("could not access %s"),
> + *path);
> + VIR_FREE(*path);
> + return -1;
> + }
> + return 0;
> +}
> +
> +# else
> +
> +int
> +virShmBuildPath(const char *name ATTRIBUTE_UNUSED,
> + char **path ATTRIBUTE_UNUSED)
> +{
> + virReportSystemError(ENOSYS, "%s",
> + _("Cannot get share memory object path on this
platform"));
> + return -2;
> +}
Why -2 instead of -1 ?
I want to make caller function know if we not support this function. But
i am not sure it is a good idea, and i will give a explanation about why
we do this in another mail.
> diff --git a/src/util/virshm.h b/src/util/virshm.h
> new file mode 100644
> index 0000000..945a541
> --- /dev/null
> +++ b/src/util/virshm.h
> +# include "internal.h"
> +# include "virutil.h"
> +# include "virobject.h"
> +
> +typedef enum {
> + VIR_SHM_TYPE_SHM = 0,
> + VIR_SHM_TYPE_SERVER,
> +
> + VIR_SHM_TYPE_LAST
> +} virShmObjectType;
I'm unclear why we need the different type constants
here ?
I want to avoid the wrong region from the different type shmem device,
here is a example:
There is a shmem device which won't use ivshmem-server and we create a
new shmobject to save it information, and then a shmem device which have
the same name with the first shmem device but its a server enabled shmem
device need region to list too, then we will try to add the second shmem
device to the first shmobject if the size is equal.
So i want to introduce a new type constants to split these different
shmem device.
> +
> +typedef struct _virShmObject virShmObject;
> +typedef virShmObject *virShmObjectPtr;
> +struct _virShmObject {
> + char *name; /* shmem object name */
> + int type; /* shmem object type */
> + unsigned long long size; /* size of shmem object */
> + char *path; /* shmem path */
> + bool othercreate; /* a bool parameter record if the shm is created
by libvirt */
> +
> + char *drvname; /* which driver */
> + char **domains; /* domain(s) using this shm */
> + size_t ndomains; /* number of useds */
> +};
> +
> +VIR_ENUM_DECL(virShmObject);
> +
> +typedef struct _virShmObjectList virShmObjectList;
> +typedef virShmObjectList *virShmObjectListPtr;
> +struct _virShmObjectList {
> + virObjectLockable parent;
> + char *stateDir;
> + size_t nshmobjs;
> + virShmObjectPtr *shmobjs;
> +};
I think of the struct definitions should be private. If
callers need access to the contents we can provide APIs
Okay, i will move them in virshm.c in next version
> +# ifdef HAVE_SHM_OPEN
> +int virShmCreate(const char *name, unsigned long long size,
> + bool outerr, bool *othercreate, mode_t mode)
> + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(4);
> +int virShmOpen(const char *name,
> + unsigned long long size,
> + mode_t mode)
> + ATTRIBUTE_NONNULL(1);
> +int virShmUnlink(const char *name)
> + ATTRIBUTE_NONNULL(1);
> +int virShmBuildPath(const char *name, char **path)
> + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
> +# else /* HAVE_SHM_OPEN */
> +# define virShmCreate(name, size, outerr, othercreate, mode) -2
> +# define virShmOpen(name, size, mode) -2
> +# define virShmUnlink(name) -2
> +# define virShmBuildPath(name, path) -2
> +
> +# endif /* HAVE_SHM_OPEN */
You can't just #define away the APIs - this will lead to undefined
symbols being referenced in the linker script which breaks on
Win32. You need to provide stub implementations. Also you need to
use virReportError() any time you return an error status, as you
already did for virShmBuildPath.
Oh, i see, i will fix them.
Thanks a lot for your review and help.
Regards,
Daniel
Thanks,
Luyao
5:13 a.m.
New subject: [libvirt] [PATCH 4/4] qemu: call the helpers in virshm.c to manage shmem device
Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
---
src/qemu/qemu_conf.h | 3 +
src/qemu/qemu_driver.c | 4 ++
src/qemu/qemu_process.c | 158 ++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 165 insertions(+)
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index 3f73929..61d3462 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -46,6 +46,7 @@
# include "virclosecallbacks.h"
# include "virhostdev.h"
# include "virfile.h"
+# include "virshm.h"
# ifdef CPU_SETSIZE /* Linux */
# define QEMUD_CPUMASK_LEN CPU_SETSIZE
@@ -235,6 +236,8 @@ struct _virQEMUDriver {
/* Immutable pointer. Unsafe APIs. XXX */
virHashTablePtr sharedDevices;
+ virShmObjectListPtr shmlist;
+
/* Immutable pointer, self-locking APIs */
virPortAllocatorPtr remotePorts;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 9dbe635..282ab45 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -778,6 +778,9 @@ qemuStateInitialize(bool privileged,
if (qemuMigrationErrorInit(qemu_driver) < 0)
goto error;
+ if (!(qemu_driver->shmlist = virShmObjectListGetDefault()))
+ goto error;
+
if (privileged) {
char *channeldir;
@@ -1087,6 +1090,7 @@ qemuStateCleanup(void)
virObjectUnref(qemu_driver->config);
virObjectUnref(qemu_driver->hostdevMgr);
virHashFree(qemu_driver->sharedDevices);
+ virObjectUnref(qemu_driver->shmlist);
virObjectUnref(qemu_driver->caps);
virQEMUCapsCacheFree(qemu_driver->qemuCapsCache);
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 1c0c734..84b3b5e 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4321,6 +4321,154 @@ qemuPrepareNVRAM(virQEMUDriverConfigPtr cfg,
}
+static int
+qemuPrepareShmemDevice(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainShmemDefPtr shmem)
+{
+ int ret = -1;
+ virShmObjectPtr tmp;
+ virShmObjectListPtr list = driver->shmlist;
+ bool othercreate = false;
+ char *path = NULL;
+ bool teardownlabel = false;
+ bool teardownshm = false;
+ int type, fd;
+
+ virObjectLock(list);
+
+ if ((tmp = virShmObjectFindByName(list, shmem->name))) {
+ if (shmem->size > tmp->size) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Shmem object %s is already exists and "
+ "size is smaller than require size"),
+ tmp->name);
+ goto cleanup;
+ }
+
+ if (virShmSetUsedDomain(tmp, QEMU_DRIVER_NAME, vm->def->name) < 0)
+ goto cleanup;
+
+ if (virShmObjectSaveState(tmp, list->stateDir) < 0)
+ goto cleanup;
+
+ virObjectUnlock(list);
+ return 0;
+ }
+
+ if (!shmem->server.enabled) {
+ if ((fd = virShmCreate(shmem->name, shmem->size, false, &othercreate,
0600)) < 0)
+ goto cleanup;
+ VIR_FORCE_CLOSE(fd);
+
+ if ((ret = virShmBuildPath(shmem->name, &path)) == -1) {
+ ignore_value(virShmUnlink(shmem->name));
+ goto cleanup;
+ } else if (ret == -2 && !othercreate) {
+ ignore_value(virShmUnlink(shmem->name));
+ }
+ type = VIR_SHM_TYPE_SHM;
+ } else {
+ if (!virFileExists(shmem->server.chr.data.nix.path)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Shmem device server socket is not exist"));
+ goto cleanup;
+ } else {
+ othercreate = true;
+ }
+ type = VIR_SHM_TYPE_SERVER;
+ }
+ teardownshm = true;
+
+ if (virSecurityManagerSetShmemLabel(driver->securityManager,
+ vm->def, shmem, path) < 0)
+ goto cleanup;
+ teardownlabel = true;
+
+ if (!(tmp = virShmObjectNew(shmem->name, shmem->size, path, type, othercreate,
+ QEMU_DRIVER_NAME, vm->def->name)))
+ goto cleanup;
+
+ if (virShmObjectSaveState(tmp, list->stateDir) < 0) {
+ virShmObjectFree(tmp);
+ goto cleanup;
+ }
+
+ if (virShmObjectListAdd(list, tmp) < 0) {
+ virShmObjectFree(tmp);
+ goto cleanup;
+ }
+
+ ret = 0;
+
+ cleanup:
+ if (ret < 0) {
+ if (teardownlabel &&
+ virSecurityManagerRestoreShmemLabel(driver->securityManager,
+ vm->def, shmem, path) < 0)
+ VIR_WARN("Unable to restore shared memory device labelling");
+
+ if (teardownshm && !shmem->server.enabled &&
+ !othercreate && virShmUnlink(shmem->name) < 0)
+ VIR_WARN("Unable to unlink shared memory object");
+ }
+ VIR_FREE(path);
+ virObjectUnlock(list);
+ return ret;
+}
+
+
+static int
+qemuCleanUpShmemDevice(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainShmemDefPtr shmem)
+{
+ virShmObjectPtr tmp;
+ virShmObjectListPtr list = driver->shmlist;
+ int ret = -1;
+
+ virObjectLock(list);
+
+ if (!(tmp = virShmObjectFindByName(list, shmem->name))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Cannot find share memory named '%s'"),
+ shmem->name);
+ goto cleanup;
+ }
+ if ((shmem->server.enabled && tmp->type != VIR_SHM_TYPE_SERVER) ||
+ (!shmem->server.enabled && tmp->type != VIR_SHM_TYPE_SHM)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Shmem object and shmem device type is not equal"));
+ goto cleanup;
+ }
+
+ if (virShmRemoveUsedDomain(tmp, QEMU_DRIVER_NAME, vm->def->name) < 0)
+ goto cleanup;
+
+ if (tmp->ndomains == 0) {
+ if (virSecurityManagerRestoreShmemLabel(driver->securityManager,
+ vm->def, shmem, tmp->path) < 0)
+ VIR_WARN("Unable to restore shared memory device labelling");
+
+ if (!shmem->server.enabled) {
+ if (!tmp->othercreate &&
+ virShmUnlink(tmp->name) < 0)
+ VIR_WARN("Unable to unlink shared memory object");
+ }
+
+ if (virShmObjectRemoveStateFile(list, tmp->name) < 0)
+ goto cleanup;
+ virShmObjectListDel(list, tmp);
+ virShmObjectFree(tmp);
+ }
+
+ ret = 0;
+ cleanup:
+ virObjectUnlock(list);
+ return ret;
+}
+
+
static void
qemuLogOperation(virDomainObjPtr vm,
const char *msg,
@@ -4753,6 +4901,11 @@ int qemuProcessStart(virConnectPtr conn,
if (cfg->clearEmulatorCapabilities)
virCommandClearCaps(cmd);
+ for (i = 0; i < vm->def->nshmems; i++) {
+ if (qemuPrepareShmemDevice(driver, vm, vm->def->shmems[i]) < 0)
+ goto cleanup;
+ }
+
/* in case a certain disk is desirous of CAP_SYS_RAWIO, add this */
for (i = 0; i < vm->def->ndisks; i++) {
virDomainDeviceDef dev;
@@ -5391,6 +5544,11 @@ void qemuProcessStop(virQEMUDriverPtr driver,
}
}
+ for (i = 0; i < vm->def->nshmems; i++) {
+ ignore_value(qemuCleanUpShmemDevice(driver, vm,
+ vm->def->shmems[i]));
+ }
+
vm->taint = 0;
vm->pid = -1;
virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, reason);
--
1.8.3.1
10:52 a.m.
New subject: [libvirt] [PATCH 4/4] qemu: call the helpers in virshm.c to manage shmem device
Hi
On Thu, Jul 23, 2015 at 12:13 PM, Luyao Huang <lhuang(a)redhat.com> wrote:
Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
---
src/qemu/qemu_conf.h | 3 +
src/qemu/qemu_driver.c | 4 ++
src/qemu/qemu_process.c | 158 ++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 165 insertions(+)
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index 3f73929..61d3462 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -46,6 +46,7 @@
# include "virclosecallbacks.h"
# include "virhostdev.h"
# include "virfile.h"
+# include "virshm.h"
# ifdef CPU_SETSIZE /* Linux */
# define QEMUD_CPUMASK_LEN CPU_SETSIZE
@@ -235,6 +236,8 @@ struct _virQEMUDriver {
/* Immutable pointer. Unsafe APIs. XXX */
virHashTablePtr sharedDevices;
+ virShmObjectListPtr shmlist;
+
/* Immutable pointer, self-locking APIs */
virPortAllocatorPtr remotePorts;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 9dbe635..282ab45 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -778,6 +778,9 @@ qemuStateInitialize(bool privileged,
if (qemuMigrationErrorInit(qemu_driver) < 0)
goto error;
+ if (!(qemu_driver->shmlist = virShmObjectListGetDefault()))
+ goto error;
+
if (privileged) {
char *channeldir;
@@ -1087,6 +1090,7 @@ qemuStateCleanup(void)
virObjectUnref(qemu_driver->config);
virObjectUnref(qemu_driver->hostdevMgr);
virHashFree(qemu_driver->sharedDevices);
+ virObjectUnref(qemu_driver->shmlist);
virObjectUnref(qemu_driver->caps);
virQEMUCapsCacheFree(qemu_driver->qemuCapsCache);
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 1c0c734..84b3b5e 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4321,6 +4321,154 @@ qemuPrepareNVRAM(virQEMUDriverConfigPtr cfg,
}
+static int
+qemuPrepareShmemDevice(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainShmemDefPtr shmem)
+{
+ int ret = -1;
+ virShmObjectPtr tmp;
+ virShmObjectListPtr list = driver->shmlist;
+ bool othercreate = false;
+ char *path = NULL;
+ bool teardownlabel = false;
+ bool teardownshm = false;
+ int type, fd;
+
+ virObjectLock(list);
+
+ if ((tmp = virShmObjectFindByName(list, shmem->name))) {
+ if (shmem->size > tmp->size) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Shmem object %s is already exists and "
+ "size is smaller than require size"),
+ tmp->name);
+ goto cleanup;
+ }
+
+ if (virShmSetUsedDomain(tmp, QEMU_DRIVER_NAME, vm->def->name) < 0)
+ goto cleanup;
+
+ if (virShmObjectSaveState(tmp, list->stateDir) < 0)
+ goto cleanup;
+
+ virObjectUnlock(list);
+ return 0;
+ }
+
+ if (!shmem->server.enabled) {
+ if ((fd = virShmCreate(shmem->name, shmem->size, false, &othercreate,
0600)) < 0)
+ goto cleanup;
+ VIR_FORCE_CLOSE(fd);
+
+ if ((ret = virShmBuildPath(shmem->name, &path)) == -1) {
+ ignore_value(virShmUnlink(shmem->name));
+ goto cleanup;
+ } else if (ret == -2 && !othercreate) {
What is ret == -2 ?
+ ignore_value(virShmUnlink(shmem->name));
+ }
+ type = VIR_SHM_TYPE_SHM;
+ } else {
+ if (!virFileExists(shmem->server.chr.data.nix.path)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Shmem device server socket is not exist"));
is not / does not
+ goto cleanup;
+ } else {
+ othercreate = true;
+ }
+ type = VIR_SHM_TYPE_SERVER;
+ }
+ teardownshm = true;
+
+ if (virSecurityManagerSetShmemLabel(driver->securityManager,
+ vm->def, shmem, path) < 0)
+ goto cleanup;
So each time a ivshmem device starts, it will potentially change the
labels. Not sure that's a good idea. Why not apply only when created?
Btw, have you considered managing shm like storage pools? Would that
bring any benefit?
+ teardownlabel = true;
+
+ if (!(tmp = virShmObjectNew(shmem->name, shmem->size, path, type,
othercreate,
+ QEMU_DRIVER_NAME, vm->def->name)))
+ goto cleanup;
+
+ if (virShmObjectSaveState(tmp, list->stateDir) < 0) {
+ virShmObjectFree(tmp);
+ goto cleanup;
+ }
+
+ if (virShmObjectListAdd(list, tmp) < 0) {
+ virShmObjectFree(tmp);
+ goto cleanup;
+ }
+
+ ret = 0;
+
+ cleanup:
+ if (ret < 0) {
+ if (teardownlabel &&
+ virSecurityManagerRestoreShmemLabel(driver->securityManager,
+ vm->def, shmem, path) < 0)
+ VIR_WARN("Unable to restore shared memory device labelling");
+
+ if (teardownshm && !shmem->server.enabled &&
+ !othercreate && virShmUnlink(shmem->name) < 0)
+ VIR_WARN("Unable to unlink shared memory object");
+ }
+ VIR_FREE(path);
+ virObjectUnlock(list);
+ return ret;
+}
+
+
+static int
+qemuCleanUpShmemDevice(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainShmemDefPtr shmem)
+{
+ virShmObjectPtr tmp;
+ virShmObjectListPtr list = driver->shmlist;
+ int ret = -1;
+
+ virObjectLock(list);
+
+ if (!(tmp = virShmObjectFindByName(list, shmem->name))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Cannot find share memory named '%s'"),
+ shmem->name);
+ goto cleanup;
+ }
+ if ((shmem->server.enabled && tmp->type != VIR_SHM_TYPE_SERVER) ||
+ (!shmem->server.enabled && tmp->type != VIR_SHM_TYPE_SHM)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Shmem object and shmem device type is not equal"));
+ goto cleanup;
+ }
+
+ if (virShmRemoveUsedDomain(tmp, QEMU_DRIVER_NAME, vm->def->name) < 0)
+ goto cleanup;
+
+ if (tmp->ndomains == 0) {
+ if (virSecurityManagerRestoreShmemLabel(driver->securityManager,
+ vm->def, shmem, tmp->path) <
0)
+ VIR_WARN("Unable to restore shared memory device labelling");
+
+ if (!shmem->server.enabled) {
+ if (!tmp->othercreate &&
+ virShmUnlink(tmp->name) < 0)
+ VIR_WARN("Unable to unlink shared memory object");
+ }
+
+ if (virShmObjectRemoveStateFile(list, tmp->name) < 0)
+ goto cleanup;
+ virShmObjectListDel(list, tmp);
+ virShmObjectFree(tmp);
+ }
+
+ ret = 0;
+ cleanup:
+ virObjectUnlock(list);
+ return ret;
+}
+
+
static void
qemuLogOperation(virDomainObjPtr vm,
const char *msg,
@@ -4753,6 +4901,11 @@ int qemuProcessStart(virConnectPtr conn,
if (cfg->clearEmulatorCapabilities)
virCommandClearCaps(cmd);
+ for (i = 0; i < vm->def->nshmems; i++) {
+ if (qemuPrepareShmemDevice(driver, vm, vm->def->shmems[i]) < 0)
+ goto cleanup;
+ }
+
/* in case a certain disk is desirous of CAP_SYS_RAWIO, add this */
for (i = 0; i < vm->def->ndisks; i++) {
virDomainDeviceDef dev;
@@ -5391,6 +5544,11 @@ void qemuProcessStop(virQEMUDriverPtr driver,
}
}
+ for (i = 0; i < vm->def->nshmems; i++) {
+ ignore_value(qemuCleanUpShmemDevice(driver, vm,
+ vm->def->shmems[i]));
+ }
+
vm->taint = 0;
vm->pid = -1;
virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, reason);
--
1.8.3.1
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
--
Marc-André Lureau
7346 2483 9404 4E20 ABFF 7D48 D864 9487 F43F 0992
1:23 a.m.
New subject: [libvirt] [PATCH 4/4] qemu: call the helpers in virshm.c to manage shmem device
Hi Marc-André
On 07/27/2015 11:52 PM, Marc-André Lureau wrote:
Hi
On Thu, Jul 23, 2015 at 12:13 PM, Luyao Huang <lhuang(a)redhat.com> wrote:
> Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
> ---
> src/qemu/qemu_conf.h | 3 +
> src/qemu/qemu_driver.c | 4 ++
> src/qemu/qemu_process.c | 158 ++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 165 insertions(+)
>
> diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
> index 3f73929..61d3462 100644
> --- a/src/qemu/qemu_conf.h
> +++ b/src/qemu/qemu_conf.h
> @@ -46,6 +46,7 @@
> # include "virclosecallbacks.h"
> # include "virhostdev.h"
> # include "virfile.h"
> +# include "virshm.h"
>
> # ifdef CPU_SETSIZE /* Linux */
> # define QEMUD_CPUMASK_LEN CPU_SETSIZE
> @@ -235,6 +236,8 @@ struct _virQEMUDriver {
> /* Immutable pointer. Unsafe APIs. XXX */
> virHashTablePtr sharedDevices;
>
> + virShmObjectListPtr shmlist;
> +
> /* Immutable pointer, self-locking APIs */
> virPortAllocatorPtr remotePorts;
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 9dbe635..282ab45 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -778,6 +778,9 @@ qemuStateInitialize(bool privileged,
> if (qemuMigrationErrorInit(qemu_driver) < 0)
> goto error;
>
> + if (!(qemu_driver->shmlist = virShmObjectListGetDefault()))
> + goto error;
> +
> if (privileged) {
> char *channeldir;
>
> @@ -1087,6 +1090,7 @@ qemuStateCleanup(void)
> virObjectUnref(qemu_driver->config);
> virObjectUnref(qemu_driver->hostdevMgr);
> virHashFree(qemu_driver->sharedDevices);
> + virObjectUnref(qemu_driver->shmlist);
> virObjectUnref(qemu_driver->caps);
> virQEMUCapsCacheFree(qemu_driver->qemuCapsCache);
>
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 1c0c734..84b3b5e 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -4321,6 +4321,154 @@ qemuPrepareNVRAM(virQEMUDriverConfigPtr cfg,
> }
>
>
> +static int
> +qemuPrepareShmemDevice(virQEMUDriverPtr driver,
> + virDomainObjPtr vm,
> + virDomainShmemDefPtr shmem)
> +{
> + int ret = -1;
> + virShmObjectPtr tmp;
> + virShmObjectListPtr list = driver->shmlist;
> + bool othercreate = false;
> + char *path = NULL;
> + bool teardownlabel = false;
> + bool teardownshm = false;
> + int type, fd;
> +
> + virObjectLock(list);
> +
> + if ((tmp = virShmObjectFindByName(list, shmem->name))) {
> + if (shmem->size > tmp->size) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Shmem object %s is already exists and "
> + "size is smaller than require size"),
> + tmp->name);
> + goto cleanup;
> + }
> +
> + if (virShmSetUsedDomain(tmp, QEMU_DRIVER_NAME, vm->def->name) < 0)
> + goto cleanup;
> +
> + if (virShmObjectSaveState(tmp, list->stateDir) < 0)
> + goto cleanup;
> +
> + virObjectUnlock(list);
> + return 0;
> + }
> +
> + if (!shmem->server.enabled) {
> + if ((fd = virShmCreate(shmem->name, shmem->size, false,
&othercreate, 0600)) < 0)
> + goto cleanup;
> + VIR_FORCE_CLOSE(fd);
> +
> + if ((ret = virShmBuildPath(shmem->name, &path)) == -1) {
> + ignore_value(virShmUnlink(shmem->name));
> + goto cleanup;
> + } else if (ret == -2 && !othercreate) {
What is ret == -2 ?
when ret = -2 this means we do not support virShmBuildPath in that
platform (this could only happened on some other platform which support
shm_open/shm_unlink ,just like solaris, freebsd), at that platform the
shm obj is not in /dev/shm or not exist in the file system, if we help
to create that shm obj but cannot find a way to relabel it, qemu will
cannot use the shm in that case, so unlink the shm and let qemu create
it will make guest can start success, and we could unlink the qemu
create shm obj if there is no guest use it.
> + ignore_value(virShmUnlink(shmem->name));
> + }
> + type = VIR_SHM_TYPE_SHM;
> + } else {
> + if (!virFileExists(shmem->server.chr.data.nix.path)) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Shmem device server socket is not exist"));
is not / does not
> + goto cleanup;
> + } else {
> + othercreate = true;
> + }
> + type = VIR_SHM_TYPE_SERVER;
> + }
> + teardownshm = true;
> +
> + if (virSecurityManagerSetShmemLabel(driver->securityManager,
> + vm->def, shmem, path) < 0)
> + goto cleanup;
So each time a ivshmem device starts, it will potentially change the
labels. Not sure that's a good idea. Why not apply only when created?
Good question, we allow specify the label in the shmem device XML, and
if the user create the shm with a label which only allow that guest
access, then the other guest will cannot use it if we do not change the
label, but if we change the label to a label which allow all libvirt
guest can access, it will make the first guest not safe.(Also this will
happen when different guest use one shared disk), i have an quick
thought to solve this issue:
Introduce the <shareable/> in the shmem device and forbid use one shm
object in different shared policy (a example: two guest use one shm obj
and one use it with a shared label and another not), and write some
document to point out this.
Btw, have you considered managing shm like storage pools? Would that
bring any benefit?
I thought about this before, and in my opinion introduce a lot of public
API (just like:list, delete, create...) is unnecessary: there are two
usage of shmem device: use it for a guest-host connect or for
guest-guest connect. if one guest is using the shmem resource
(ivshmem-server/shm obj), we should make sure we won't remove the
resource. and after no guest use it we will clean up the resource. Also
we provide a way to keep the resource here (we still will relabel the
resource) after all guest is not use it, and users could clean the
resource when they want. As the requirement for set up and clean up
shmem device resource is very clear, we could help users to do this(the
users can just use it without prepare extra code to find a way to know
when need set up/clean up the resource).
Thanks a lot for your review and help.
Luyao
5:23 a.m.
New subject: [libvirt] [PATCH 4/4] qemu: call the helpers in virshm.c to manage shmem device
On Thu, Jul 23, 2015 at 06:13:49PM +0800, Luyao Huang wrote:
Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
---
src/qemu/qemu_conf.h | 3 +
src/qemu/qemu_driver.c | 4 ++
src/qemu/qemu_process.c | 158 ++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 165 insertions(+)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 1c0c734..84b3b5e 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4321,6 +4321,154 @@ qemuPrepareNVRAM(virQEMUDriverConfigPtr cfg,
}
+static int
+qemuPrepareShmemDevice(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainShmemDefPtr shmem)
+{
+ int ret = -1;
+ virShmObjectPtr tmp;
+ virShmObjectListPtr list = driver->shmlist;
+ bool othercreate = false;
+ char *path = NULL;
+ bool teardownlabel = false;
+ bool teardownshm = false;
+ int type, fd;
+
+ virObjectLock(list);
+
+ if ((tmp = virShmObjectFindByName(list, shmem->name))) {
+ if (shmem->size > tmp->size) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Shmem object %s is already exists and "
+ "size is smaller than require size"),
+ tmp->name);
+ goto cleanup;
+ }
+
+ if (virShmSetUsedDomain(tmp, QEMU_DRIVER_NAME, vm->def->name) < 0)
+ goto cleanup;
+
+ if (virShmObjectSaveState(tmp, list->stateDir) < 0)
+ goto cleanup;
+
+ virObjectUnlock(list);
+ return 0;
+ }
+
+ if (!shmem->server.enabled) {
+ if ((fd = virShmCreate(shmem->name, shmem->size, false, &othercreate,
0600)) < 0)
+ goto cleanup;
+ VIR_FORCE_CLOSE(fd);
+
+ if ((ret = virShmBuildPath(shmem->name, &path)) == -1) {
+ ignore_value(virShmUnlink(shmem->name));
+ goto cleanup;
+ } else if (ret == -2 && !othercreate) {
+ ignore_value(virShmUnlink(shmem->name));
+ }
+ type = VIR_SHM_TYPE_SHM;
+ } else {
+ if (!virFileExists(shmem->server.chr.data.nix.path)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Shmem device server socket is not exist"));
+ goto cleanup;
+ } else {
+ othercreate = true;
+ }
+ type = VIR_SHM_TYPE_SERVER;
+ }
+ teardownshm = true;
+
+ if (virSecurityManagerSetShmemLabel(driver->securityManager,
+ vm->def, shmem, path) < 0)
+ goto cleanup;
You shouldn't be setting labelling at this point. That should be done
by the later call to virSecurityManagerSetAllLabel
+static int
+qemuCleanUpShmemDevice(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainShmemDefPtr shmem)
+{
+ virShmObjectPtr tmp;
+ virShmObjectListPtr list = driver->shmlist;
+ int ret = -1;
+
+ virObjectLock(list);
+
+ if (!(tmp = virShmObjectFindByName(list, shmem->name))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Cannot find share memory named '%s'"),
+ shmem->name);
+ goto cleanup;
+ }
+ if ((shmem->server.enabled && tmp->type != VIR_SHM_TYPE_SERVER) ||
+ (!shmem->server.enabled && tmp->type != VIR_SHM_TYPE_SHM)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Shmem object and shmem device type is not equal"));
+ goto cleanup;
+ }
+
+ if (virShmRemoveUsedDomain(tmp, QEMU_DRIVER_NAME, vm->def->name) < 0)
+ goto cleanup;
+
+ if (tmp->ndomains == 0) {
+ if (virSecurityManagerRestoreShmemLabel(driver->securityManager,
+ vm->def, shmem, tmp->path) <
0)
+ VIR_WARN("Unable to restore shared memory device labelling");
Likewise this should be left to the main label restore code
+
+ if (!shmem->server.enabled) {
+ if (!tmp->othercreate &&
+ virShmUnlink(tmp->name) < 0)
+ VIR_WARN("Unable to unlink shared memory object");
+ }
+
+ if (virShmObjectRemoveStateFile(list, tmp->name) < 0)
+ goto cleanup;
+ virShmObjectListDel(list, tmp);
+ virShmObjectFree(tmp);
+ }
+
+ ret = 0;
+ cleanup:
+ virObjectUnlock(list);
+ return ret;
+}
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 :|
1:22 a.m.
New subject: [libvirt] [PATCH 4/4] qemu: call the helpers in virshm.c to manage shmem device
On 07/30/2015 06:23 PM, Daniel P. Berrange wrote:
On Thu, Jul 23, 2015 at 06:13:49PM +0800, Luyao Huang wrote:
> Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
> ---
> src/qemu/qemu_conf.h | 3 +
> src/qemu/qemu_driver.c | 4 ++
> src/qemu/qemu_process.c | 158 ++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 165 insertions(+)
>
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 1c0c734..84b3b5e 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -4321,6 +4321,154 @@ qemuPrepareNVRAM(virQEMUDriverConfigPtr cfg,
> }
>
>
> +static int
> +qemuPrepareShmemDevice(virQEMUDriverPtr driver,
> + virDomainObjPtr vm,
> + virDomainShmemDefPtr shmem)
> +{
> + int ret = -1;
> + virShmObjectPtr tmp;
> + virShmObjectListPtr list = driver->shmlist;
> + bool othercreate = false;
> + char *path = NULL;
> + bool teardownlabel = false;
> + bool teardownshm = false;
> + int type, fd;
> +
> + virObjectLock(list);
> +
> + if ((tmp = virShmObjectFindByName(list, shmem->name))) {
> + if (shmem->size > tmp->size) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Shmem object %s is already exists and "
> + "size is smaller than require size"),
> + tmp->name);
> + goto cleanup;
> + }
> +
> + if (virShmSetUsedDomain(tmp, QEMU_DRIVER_NAME, vm->def->name) < 0)
> + goto cleanup;
> +
> + if (virShmObjectSaveState(tmp, list->stateDir) < 0)
> + goto cleanup;
> +
> + virObjectUnlock(list);
> + return 0;
> + }
> +
> + if (!shmem->server.enabled) {
> + if ((fd = virShmCreate(shmem->name, shmem->size, false,
&othercreate, 0600)) < 0)
> + goto cleanup;
> + VIR_FORCE_CLOSE(fd);
> +
> + if ((ret = virShmBuildPath(shmem->name, &path)) == -1) {
> + ignore_value(virShmUnlink(shmem->name));
> + goto cleanup;
> + } else if (ret == -2 && !othercreate) {
> + ignore_value(virShmUnlink(shmem->name));
> + }
> + type = VIR_SHM_TYPE_SHM;
> + } else {
> + if (!virFileExists(shmem->server.chr.data.nix.path)) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Shmem device server socket is not exist"));
> + goto cleanup;
> + } else {
> + othercreate = true;
> + }
> + type = VIR_SHM_TYPE_SERVER;
> + }
> + teardownshm = true;
> +
> + if (virSecurityManagerSetShmemLabel(driver->securityManager,
> + vm->def, shmem, path) < 0)
> + goto cleanup;
You shouldn't be setting labelling at this point. That should be done
by the later call to virSecurityManagerSetAllLabel
Okay, i see, i will move it to virSecurityManagerSetAllLabel
> +static int
> +qemuCleanUpShmemDevice(virQEMUDriverPtr driver,
> + virDomainObjPtr vm,
> + virDomainShmemDefPtr shmem)
> +{
> + virShmObjectPtr tmp;
> + virShmObjectListPtr list = driver->shmlist;
> + int ret = -1;
> +
> + virObjectLock(list);
> +
> + if (!(tmp = virShmObjectFindByName(list, shmem->name))) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Cannot find share memory named '%s'"),
> + shmem->name);
> + goto cleanup;
> + }
> + if ((shmem->server.enabled && tmp->type != VIR_SHM_TYPE_SERVER)
||
> + (!shmem->server.enabled && tmp->type != VIR_SHM_TYPE_SHM)) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Shmem object and shmem device type is not
equal"));
> + goto cleanup;
> + }
> +
> + if (virShmRemoveUsedDomain(tmp, QEMU_DRIVER_NAME, vm->def->name) < 0)
> + goto cleanup;
> +
> + if (tmp->ndomains == 0) {
> + if (virSecurityManagerRestoreShmemLabel(driver->securityManager,
> + vm->def, shmem, tmp->path)
< 0)
> + VIR_WARN("Unable to restore shared memory device labelling");
Likewise this should be left to the main label restore code
Okay, Thanks a lot for your review and advise.
> +
> + if (!shmem->server.enabled) {
> + if (!tmp->othercreate &&
> + virShmUnlink(tmp->name) < 0)
> + VIR_WARN("Unable to unlink shared memory object");
> + }
> +
> + if (virShmObjectRemoveStateFile(list, tmp->name) < 0)
> + goto cleanup;
> + virShmObjectListDel(list, tmp);
> + virShmObjectFree(tmp);
> + }
> +
> + ret = 0;
> + cleanup:
> + virObjectUnlock(list);
> + return ret;
> +}
Regards,
Daniel
Luyao
5:25 a.m.
New subject: [libvirt] [PATCH 4/4] qemu: call the helpers in virshm.c to manage shmem device
On Thu, Jul 23, 2015 at 06:13:49PM +0800, Luyao Huang wrote:
Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
---
src/qemu/qemu_conf.h | 3 +
src/qemu/qemu_driver.c | 4 ++
src/qemu/qemu_process.c | 158 ++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 165 insertions(+)
+static int
+qemuPrepareShmemDevice(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainShmemDefPtr shmem)
+{
+ int ret = -1;
+ virShmObjectPtr tmp;
+ virShmObjectListPtr list = driver->shmlist;
+ bool othercreate = false;
+ char *path = NULL;
+ bool teardownlabel = false;
+ bool teardownshm = false;
+ int type, fd;
+
+ virObjectLock(list);
+
+ if ((tmp = virShmObjectFindByName(list, shmem->name))) {
+ if (shmem->size > tmp->size) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Shmem object %s is already exists and "
+ "size is smaller than require size"),
+ tmp->name);
+ goto cleanup;
+ }
+
+ if (virShmSetUsedDomain(tmp, QEMU_DRIVER_NAME, vm->def->name) < 0)
+ goto cleanup;
+
+ if (virShmObjectSaveState(tmp, list->stateDir) < 0)
+ goto cleanup;
+
+ virObjectUnlock(list);
+ return 0;
+ }
+
+ if (!shmem->server.enabled) {
+ if ((fd = virShmCreate(shmem->name, shmem->size, false, &othercreate,
0600)) < 0)
+ goto cleanup;
+ VIR_FORCE_CLOSE(fd);
+
+ if ((ret = virShmBuildPath(shmem->name, &path)) == -1) {
+ ignore_value(virShmUnlink(shmem->name));
+ goto cleanup;
+ } else if (ret == -2 && !othercreate) {
+ ignore_value(virShmUnlink(shmem->name));
Why are you treating -1 differentl from -2 - in both cases we should
abort creation as that indicates the method either failed or is not
supported in this platform.
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 :|
2:47 a.m.
New subject: [libvirt] [PATCH 4/4] qemu: call the helpers in virshm.c to manage shmem device
On 07/30/2015 06:25 PM, Daniel P. Berrange wrote:
On Thu, Jul 23, 2015 at 06:13:49PM +0800, Luyao Huang wrote:
> Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
> ---
> src/qemu/qemu_conf.h | 3 +
> src/qemu/qemu_driver.c | 4 ++
> src/qemu/qemu_process.c | 158 ++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 165 insertions(+)
>
> +static int
> +qemuPrepareShmemDevice(virQEMUDriverPtr driver,
> + virDomainObjPtr vm,
> + virDomainShmemDefPtr shmem)
> +{
> + int ret = -1;
> + virShmObjectPtr tmp;
> + virShmObjectListPtr list = driver->shmlist;
> + bool othercreate = false;
> + char *path = NULL;
> + bool teardownlabel = false;
> + bool teardownshm = false;
> + int type, fd;
> +
> + virObjectLock(list);
> +
> + if ((tmp = virShmObjectFindByName(list, shmem->name))) {
> + if (shmem->size > tmp->size) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Shmem object %s is already exists and "
> + "size is smaller than require size"),
> + tmp->name);
> + goto cleanup;
> + }
> +
> + if (virShmSetUsedDomain(tmp, QEMU_DRIVER_NAME, vm->def->name) < 0)
> + goto cleanup;
> +
> + if (virShmObjectSaveState(tmp, list->stateDir) < 0)
> + goto cleanup;
> +
> + virObjectUnlock(list);
> + return 0;
> + }
> +
> + if (!shmem->server.enabled) {
> + if ((fd = virShmCreate(shmem->name, shmem->size, false,
&othercreate, 0600)) < 0)
> + goto cleanup;
> + VIR_FORCE_CLOSE(fd);
> +
> + if ((ret = virShmBuildPath(shmem->name, &path)) == -1) {
> + ignore_value(virShmUnlink(shmem->name));
> + goto cleanup;
> + } else if (ret == -2 && !othercreate) {
> + ignore_value(virShmUnlink(shmem->name));
Why are you treating -1 differentl from -2 - in both cases we should
abort creation as that indicates the method either failed or is not
supported in this platform.
What i thought when i wrote this is : when ret = -2 this means we do not
support virShmBuildPath in that platform (this could only happened on
some other platform which support shm_open/shm_unlink ,just like
solaris, freebsd) but we could use shm_open, on that platform the shm
obj is not in /dev/shm or not exist in the file system, if we help to
create that shm obj but cannot find a way to relabel it, qemu will
cannot use the shm in that case, so unlink the shm and let qemu create
it will make guest can start success, and we could unlink the qemu
create shm obj if there is no guest use it.
I am not sure this is a good idea right now, since i am not sure this
will work as except on different platform. Maybe i should remove it and
make virShmBuildPath return -1 if not support on that platform.
Regards,
Daniel
Luyao
9:09 a.m.
Hi Luyao
On Thu, Jul 23, 2015 at 12:13 PM, Luyao Huang <lhuang(a)redhat.com> wrote:
But the problem is we cannot change the running shm-server process
label,
We need wait ivshmem-server to be a part of qemu progrem, then setup the
ivshmem-server by libvirt. we cannot do nothing for the ivshmem-server right now.
ivshmem-server is going to be a separate server process, not part of
qemu process.
Is it enough if ivshmem-server is started by libvirt to solve the selinux issue?
What's missing to get started to support it with libvirt?
--
Marc-André Lureau
9:40 a.m.
On Mon, Jul 27, 2015 at 04:09:01PM +0200, Marc-André Lureau wrote:
Hi Luyao
On Thu, Jul 23, 2015 at 12:13 PM, Luyao Huang <lhuang(a)redhat.com> wrote:
> But the problem is we cannot change the running shm-server process label,
> We need wait ivshmem-server to be a part of qemu progrem, then setup the
> ivshmem-server by libvirt. we cannot do nothing for the ivshmem-server right now.
ivshmem-server is going to be a separate server process, not part of
qemu process.
Is it enough if ivshmem-server is started by libvirt to solve the selinux issue?
What's missing to get started to support it with libvirt?
The complexity arises when multiple QEMUs want to connect to the same
memory region. Each QEMU has its own unique SELinux label (eg something
like svirt_t:s0:c352,c850 with random category values) So there is
no single SElinux label you can give to an ivshmem server process to
let it "just work" with multiple QEMUs, unless we chose to effectively
just let any QEMU connect whatsoever by running ivmshmem-server under
svirt_t:s0:c0.c1023 which removes all isolation between the guests.
This is label we use for disk images which must be shared between
QEMUs currently, but long term we're going to need to come up with
a way to allow concurrent access but kep separation. At that point
we'll likely need to implement the ivmshmem server as part of the
libvirt project itself, so we can deal with SELinux.
Until that point though, I think the simplest thing todo is to get
an addition to the SELinux policy. We want to have
- ivshmem-server running under a 'qemu_shmemd_t' type
- ivshmem-server UNIX domain socket labeled 'qemu_shmemd_sock_t'
- svirt_t permitted to connect to any qemu_shmemd_sock_t
this doesn't require any code in libvirt or QEMU - should be possible
todo it entirely in selinux policy rules.
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 :|
10:24 a.m.
On Mon, Jul 27, 2015 at 03:40:36PM +0100, Daniel P. Berrange wrote:
On Mon, Jul 27, 2015 at 04:09:01PM +0200, Marc-André Lureau wrote:
> Hi Luyao
>
> On Thu, Jul 23, 2015 at 12:13 PM, Luyao Huang <lhuang(a)redhat.com> wrote:
> > But the problem is we cannot change the running shm-server process label,
> > We need wait ivshmem-server to be a part of qemu progrem, then setup the
> > ivshmem-server by libvirt. we cannot do nothing for the ivshmem-server right
now.
>
> ivshmem-server is going to be a separate server process, not part of
> qemu process.
>
> Is it enough if ivshmem-server is started by libvirt to solve the selinux issue?
>
> What's missing to get started to support it with libvirt?
The complexity arises when multiple QEMUs want to connect to the same
memory region. Each QEMU has its own unique SELinux label (eg something
like svirt_t:s0:c352,c850 with random category values) So there is
no single SElinux label you can give to an ivshmem server process to
let it "just work" with multiple QEMUs, unless we chose to effectively
just let any QEMU connect whatsoever by running ivmshmem-server under
svirt_t:s0:c0.c1023 which removes all isolation between the guests.
This is label we use for disk images which must be shared between
QEMUs currently, but long term we're going to need to come up with
a way to allow concurrent access but kep separation. At that point
we'll likely need to implement the ivmshmem server as part of the
libvirt project itself, so we can deal with SELinux.
I think we can say this won't "just work" unless the user sets
<seclabel> for the shmem properly or will turn off relabelling.
Libvirt must be the one to create the shm in order to properly audit
it and in case the shm is accessible by multiple QEMU instances, then
that needs to be audited as well. That's according to security guys,
because that deals with the separation problem the same way as with
shared disks for example.
Until that point though, I think the simplest thing todo is to get
an addition to the SELinux policy. We want to have
- ivshmem-server running under a 'qemu_shmemd_t' type
- ivshmem-server UNIX domain socket labeled 'qemu_shmemd_sock_t'
- svirt_t permitted to connect to any qemu_shmemd_sock_t
this doesn't require any code in libvirt or QEMU - should be possible
todo it entirely in selinux policy rules.
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 :|
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
11:05 a.m.
On Mon, Jul 27, 2015 at 03:40:36PM +0100, Daniel P. Berrange wrote:
On Mon, Jul 27, 2015 at 04:09:01PM +0200, Marc-André Lureau wrote:
> Hi Luyao
>
> On Thu, Jul 23, 2015 at 12:13 PM, Luyao Huang <lhuang(a)redhat.com> wrote:
> > But the problem is we cannot change the running shm-server process label,
> > We need wait ivshmem-server to be a part of qemu progrem, then setup the
> > ivshmem-server by libvirt. we cannot do nothing for the ivshmem-server right
now.
>
> ivshmem-server is going to be a separate server process, not part of
> qemu process.
>
> Is it enough if ivshmem-server is started by libvirt to solve the selinux issue?
>
> What's missing to get started to support it with libvirt?
The complexity arises when multiple QEMUs want to connect to the same
memory region. Each QEMU has its own unique SELinux label (eg something
like svirt_t:s0:c352,c850 with random category values) So there is
no single SElinux label you can give to an ivshmem server process to
let it "just work" with multiple QEMUs, unless we chose to effectively
just let any QEMU connect whatsoever by running ivmshmem-server under
svirt_t:s0:c0.c1023 which removes all isolation between the guests.
This is label we use for disk images which must be shared between
QEMUs currently, but long term we're going to need to come up with
a way to allow concurrent access but kep separation. At that point
we'll likely need to implement the ivmshmem server as part of the
libvirt project itself, so we can deal with SELinux.
Until that point though, I think the simplest thing todo is to get
an addition to the SELinux policy. We want to have
- ivshmem-server running under a 'qemu_shmemd_t' type
- ivshmem-server UNIX domain socket labeled 'qemu_shmemd_sock_t'
- svirt_t permitted to connect to any qemu_shmemd_sock_t
this doesn't require any code in libvirt or QEMU - should be possible
todo it entirely in selinux policy rules.
Just for clarification - this means I am NACK'ing all 4 patches
here, because I don't think any of this extra code is needed.
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 :|
5:43 a.m.
On Mon, Jul 27, 2015 at 6:05 PM, Daniel P. Berrange <berrange(a)redhat.com> wrote:
> > Is it enough if ivshmem-server is started by libvirt to
solve the selinux issue?
> >
> > What's missing to get started to support it with libvirt?
>
> The complexity arises when multiple QEMUs want to connect to the same
> memory region. Each QEMU has its own unique SELinux label (eg something
> like svirt_t:s0:c352,c850 with random category values) So there is
> no single SElinux label you can give to an ivshmem server process to
> let it "just work" with multiple QEMUs, unless we chose to effectively
> just let any QEMU connect whatsoever by running ivmshmem-server under
> svirt_t:s0:c0.c1023 which removes all isolation between the guests.
> This is label we use for disk images which must be shared between
> QEMUs currently, but long term we're going to need to come up with
> a way to allow concurrent access but kep separation. At that point
> we'll likely need to implement the ivmshmem server as part of the
> libvirt project itself, so we can deal with SELinux.
Could we start with a simple support, like with disk sharing? Tthe
problem is similar, so it's unfair to give ivshmem more requirements
than we do with disks. Furthermore, it's likely that the solution will
be similar for both, so this could be treated seperately for both.
>
> Until that point though, I think the simplest thing todo is to get
> an addition to the SELinux policy. We want to have
>
> - ivshmem-server running under a 'qemu_shmemd_t' type
> - ivshmem-server UNIX domain socket labeled 'qemu_shmemd_sock_t'
> - svirt_t permitted to connect to any qemu_shmemd_sock_t
>
That looks compatible with Luyao patches.
> this doesn't require any code in libvirt or QEMU - should be
possible
> todo it entirely in selinux policy rules.
Just for clarification - this means I am NACK'ing all 4 patches
here, because I don't think any of this extra code is needed.
Luyao patches are also about keeping track and cleaning up zombie shm.
This is useful in various cases, especially when running ivshmem-vms
without ivhsmem server.
The security part permits specifying different labels, like with disk.
Why wouldn't that be useful? (when the shm is not managed by a ivshmem
server for ex).
--
Marc-André Lureau
1:06 a.m.
On 07/28/2015 12:05 AM, Daniel P. Berrange wrote:
On Mon, Jul 27, 2015 at 03:40:36PM +0100, Daniel P. Berrange wrote:
> On Mon, Jul 27, 2015 at 04:09:01PM +0200, Marc-André Lureau wrote:
>> Hi Luyao
>>
>> On Thu, Jul 23, 2015 at 12:13 PM, Luyao Huang<lhuang(a)redhat.com> wrote:
>>> But the problem is we cannot change the running shm-server process label,
>>> We need wait ivshmem-server to be a part of qemu progrem, then setup the
>>> ivshmem-server by libvirt. we cannot do nothing for the ivshmem-server right
now.
>> ivshmem-server is going to be a separate server process, not part of
>> qemu process.
>>
>> Is it enough if ivshmem-server is started by libvirt to solve the selinux issue?
>>
>> What's missing to get started to support it with libvirt?
> The complexity arises when multiple QEMUs want to connect to the same
> memory region. Each QEMU has its own unique SELinux label (eg something
> like svirt_t:s0:c352,c850 with random category values) So there is
> no single SElinux label you can give to an ivshmem server process to
> let it "just work" with multiple QEMUs, unless we chose to effectively
> just let any QEMU connect whatsoever by running ivmshmem-server under
> svirt_t:s0:c0.c1023 which removes all isolation between the guests.
> This is label we use for disk images which must be shared between
> QEMUs currently, but long term we're going to need to come up with
> a way to allow concurrent access but kep separation. At that point
> we'll likely need to implement the ivmshmem server as part of the
> libvirt project itself, so we can deal with SELinux.
>
> Until that point though, I think the simplest thing todo is to get
> an addition to the SELinux policy. We want to have
>
> - ivshmem-server running under a 'qemu_shmemd_t' type
> - ivshmem-server UNIX domain socket labeled 'qemu_shmemd_sock_t'
> - svirt_t permitted to connect to any qemu_shmemd_sock_t
>
> this doesn't require any code in libvirt or QEMU - should be possible
> todo it entirely in selinux policy rules.
Just for clarification - this means I am NACK'ing all 4 patches
here, because I don't think any of this extra code is needed.
libvirt still need fix the issues when use shmem device without
server(that is why i want manage the shmem resource ):
1. qemu won't unlink the share memory object after exit, and when we
restart the guest which use the old
shmem device name but size is bigger than old shmem device, qemu will
forbid guest start. then user need unlink the shm obj by themselves or
give a new name to the shmem device (this will leak the shm obj).
2. if let qemu create shm obj the label will depend on qemu process
label and like this:
-rwxrwxr-x. qemu qemu system_u:object_r:svirt_tmpfs_t:s0 my_shmem1
This label is not under libvirt control, and if the user want only one
guest use this (for host-guest connect) , the label is not good enough.
3. it will be better if we offer a way to users to specify the label by
themselves, because we cannot know how the user will use the shmem
device, so the label maybe not correct in some case.
So i wrote some helper to manage the shmem device resource to solve 3
issues, ivshmem server selinux label is a small part of them, i could
remove that part in a new version, i think the first and second issue
still need fix.
Thanks a lot for your review and advise.
Regards,
Daniel
Luyao
1:19 a.m.
Hi Marc-André
On 07/27/2015 10:09 PM, Marc-André Lureau wrote:
Hi Luyao
On Thu, Jul 23, 2015 at 12:13 PM, Luyao Huang <lhuang(a)redhat.com> wrote:
> But the problem is we cannot change the running shm-server process label,
> We need wait ivshmem-server to be a part of qemu progrem, then setup the
> ivshmem-server by libvirt. we cannot do nothing for the ivshmem-server right now.
ivshmem-server is going to be a separate server process, not part of
qemu process.
OKay, thanks for clarification, i saw the patches in qemu-devel mail
list, so i thought it will be a part of qemu project :)
Is it enough if ivshmem-server is started by libvirt to solve the
selinux issue?
I think it is enough, also as Daniel's reply: it will be better if
ivshmem-server could be a part of libvirt.
What's missing to get started to support it with libvirt?
I think is the ivshmem-server set up and audit part, and i think Martin
will be the best people to give a answer.
Luyao
9:21 a.m.
On Thu, Jul 23, 2015 at 06:13:45PM +0800, Luyao Huang wrote:
Since there is a shmobj leak when let qemu create shmobj by
themselves, also the label of shmobj/shmem-server socket
is not right. Guest cannot direct use the shmem-server
if users enabled selinux. So it will be better to manage it
in libvirt.
The way i chosed is region the shmem deivce in a list, and
save it status to a local file to avoid losing it after restart
libvirtd, and count the guest which use it, and let the callers
know if there is no guest is using it (then we can relabel/cleanup
some resource).
Notice: you still cannot use the ivshmem-server if the process label
is not correct, just set the socket label is not enought, selinux
still will forbid qemu use it, because the shmem-server's process is
not correct, you will find the AVC like this (i set up the ivshmem
server via shell):
type=AVC msg=audit(1437642157.227:73784): avc: denied { connectto } for \
pid=6137 comm="qemu-kvm" path="/tmp/ivshmem_socket" \
scontext=system_u:system_r:svirt_t:s0:c703,c707 \
tcontext=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
tclass=unix_stream_socket
But the problem is we cannot change the running shm-server process label,
We need wait ivshmem-server to be a part of qemu progrem, then setup the
ivshmem-server by libvirt. we cannot do nothing for the ivshmem-server right now.
We really don't want ivmshmem-server to be part of the QEMU process.
IIUC, it is valid for a single server to be associated with multiple
QEMU processes, so there is no single label that can be used there.
Ultimately I don't think the QEMU shm-server is particularly suitable
for libvirt use long fine. It is a nice proof of concept but it is
missing alot of important stuff, especially the SELinux integration.
My intent at some point is to create a virtshmemd server as part of
the libvirt project which speaks the same protocol but SELinux MAC
checking for client QEMUs connecting to a particular ivmshmem segment.
It also needs stuff like exec-based restart so we can do live upgrades
of the server without interrupting service to QEMU. Doing it as part
of libvirt means we can also take advantage of all the other facilities
we have around our RPC system, such as the new libvirt-admin API and
the logging debugging infrastructure.
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 :|
3433
days inactive
3444
days old
34 comments
5 participants
participants (5)
-
Daniel P. Berrange -
lhuang -
Luyao Huang
-
Marc-André Lureau -
Martin Kletzander