Add option to preserve owner when creating an entry in Xen Store. This
may be needed in cases when Qemu is working as device model in a
domain that is Domain-0, e.g. in driver domain.
"owner" parameter for qemu_xen_xs_create() function can have special
value XS_PRESERVE_OWNER, which will make specific implementation to
get original owner of an entry and pass it back to
set_permissions() call.
Please note, that XenStore inherits permissions, so even if entry is
newly created by, it already has the owner set to match owner of entry
at previous level.
Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
--
In v2:
- Pass transaction to xs_get_permissions() in libxenstore_create()
- Added comment before XS_PRESERVE_OWNER defintion
- Extended the commit message
---
hw/i386/kvm/xen_xenstore.c | 18 ++++++++++++++++++
hw/xen/xen-operations.c | 12 ++++++++++++
include/hw/xen/xen_backend_ops.h | 7 +++++++
3 files changed, 37 insertions(+)
diff --git a/hw/i386/kvm/xen_xenstore.c b/hw/i386/kvm/xen_xenstore.c
index 6e651960b3..d0fd5d4681 100644
--- a/hw/i386/kvm/xen_xenstore.c
+++ b/hw/i386/kvm/xen_xenstore.c
@@ -1595,6 +1595,24 @@ static bool xs_be_create(struct qemu_xs_handle *h, xs_transaction_t t,
return false;
}
+ if (owner == XS_PRESERVE_OWNER) {
+ GList *prev_perms;
+ char letter;
+
+ err = xs_impl_get_perms(h->impl, 0, t, path, &prev_perms);
+ if (err) {
+ errno = err;
+ return false;
+ }
+
+ if (sscanf(prev_perms->data, "%c%u", &letter, &owner) != 2) {
+ errno = EFAULT;
+ g_list_free_full(prev_perms, g_free);
+ return false;
+ }
+ g_list_free_full(prev_perms, g_free);
+ }
+
perms_list = g_list_append(perms_list,
xs_perm_as_string(XS_PERM_NONE, owner));
perms_list = g_list_append(perms_list,
diff --git a/hw/xen/xen-operations.c b/hw/xen/xen-operations.c
index e00983ec44..ae8265635f 100644
--- a/hw/xen/xen-operations.c
+++ b/hw/xen/xen-operations.c
@@ -300,6 +300,18 @@ static bool libxenstore_create(struct qemu_xs_handle *h, xs_transaction_t t,
return false;
}
+ if (owner == XS_PRESERVE_OWNER) {
+ struct xs_permissions *tmp;
+ unsigned int num;
+
+ tmp = xs_get_permissions(h->xsh, t, path, &num);
+ if (tmp == NULL) {
+ return false;
+ }
+ perms_list[0].id = tmp[0].id;
+ free(tmp);
+ }
+
return xs_set_permissions(h->xsh, t, path, perms_list,
ARRAY_SIZE(perms_list));
}
diff --git a/include/hw/xen/xen_backend_ops.h b/include/hw/xen/xen_backend_ops.h
index 90cca85f52..79021538a3 100644
--- a/include/hw/xen/xen_backend_ops.h
+++ b/include/hw/xen/xen_backend_ops.h
@@ -266,6 +266,13 @@ typedef uint32_t xs_transaction_t;
#define XS_PERM_READ 0x01
#define XS_PERM_WRITE 0x02
+/*
+ * This is QEMU-specific special value used only by QEMU wrappers
+ * around XenStore. It can be passed to qemu_xen_xs_create() to
+ * inherit owner value from higher-level XS entry.
+ */
+#define XS_PRESERVE_OWNER 0xFFFE
+
struct xenstore_backend_ops {
struct qemu_xs_handle *(*open)(void);
void (*close)(struct qemu_xs_handle *h);
--
2.42.0On Tue, 2023-11-21 at 22:10 +0000, Volodymyr Babchuk wrote:
>
> --- a/hw/xen/xen-operations.c
> +++ b/hw/xen/xen-operations.c
> @@ -300,6 +300,18 @@ static bool libxenstore_create(struct qemu_xs_handle *h, xs_transaction_t t,
> return false;
> }
>
> + if (owner == XS_PRESERVE_OWNER) {
> + struct xs_permissions *tmp;
> + unsigned int num;
> +
> + tmp = xs_get_permissions(h->xsh, t, path, &num);
> + if (tmp == NULL) {
> + return false;
> + }
> + perms_list[0].id = tmp[0].id;
> + free(tmp);
> + }
> +
> return xs_set_permissions(h->xsh, t, path, perms_list,
> ARRAY_SIZE(perms_list));
> }
If the existing transaction is XBT_NULL I think you want to create a
new transaction for it, don't you?
Hi David,
David Woodhouse <dwmw2@infradead.org> writes:
> [[S/MIME Signed Part:Undecided]]
> On Tue, 2023-11-21 at 22:10 +0000, Volodymyr Babchuk wrote:
>>
>> --- a/hw/xen/xen-operations.c
>> +++ b/hw/xen/xen-operations.c
>> @@ -300,6 +300,18 @@ static bool libxenstore_create(struct qemu_xs_handle *h, xs_transaction_t t,
>> return false;
>> }
>>
>> + if (owner == XS_PRESERVE_OWNER) {
>> + struct xs_permissions *tmp;
>> + unsigned int num;
>> +
>> + tmp = xs_get_permissions(h->xsh, t, path, &num);
>> + if (tmp == NULL) {
>> + return false;
>> + }
>> + perms_list[0].id = tmp[0].id;
>> + free(tmp);
>> + }
>> +
>> return xs_set_permissions(h->xsh, t, path, perms_list,
>> ARRAY_SIZE(perms_list));
>> }
>
> If the existing transaction is XBT_NULL I think you want to create a
> new transaction for it, don't you?
I must say that your comment is valid even without my
changes. xenstore_mkdir() calls qemu_xen_xs_create, providing just plain
"0" (not even XBT_NULL) as a transaction, but actual xenstore interface
implementation, like xs_be_create(), issue multiple calls to lower
layer, passing "t" downwards. For example, xs_be_create() calls
xs_impl_read, xs_impl_write and xs_impl_set_perms(). If called from
xesntore_mkdir(), those three operations will be non-atomic. I don't
know if this can lead to a problem, because apparently it was so for a
long time...
--
WBR, Volodymyr
Hi David,
David Woodhouse <dwmw2@infradead.org> writes:
> [[S/MIME Signed Part:Undecided]]
> On Tue, 2023-11-21 at 22:10 +0000, Volodymyr Babchuk wrote:
>>
>> --- a/hw/xen/xen-operations.c
>> +++ b/hw/xen/xen-operations.c
>> @@ -300,6 +300,18 @@ static bool libxenstore_create(struct qemu_xs_handle *h, xs_transaction_t t,
>> return false;
>> }
>>
>> + if (owner == XS_PRESERVE_OWNER) {
>> + struct xs_permissions *tmp;
>> + unsigned int num;
>> +
>> + tmp = xs_get_permissions(h->xsh, t, path, &num);
>> + if (tmp == NULL) {
>> + return false;
>> + }
>> + perms_list[0].id = tmp[0].id;
>> + free(tmp);
>> + }
>> +
>> return xs_set_permissions(h->xsh, t, path, perms_list,
>> ARRAY_SIZE(perms_list));
>> }
>
> If the existing transaction is XBT_NULL I think you want to create a
> new transaction for it, don't you?
As per Stefano's and Paul's comments I'll drop this patch
completely. Thanks for review, thought.
--
WBR, Volodymyr
On 21/11/2023 22:10, Volodymyr Babchuk wrote: > Add option to preserve owner when creating an entry in Xen Store. This > may be needed in cases when Qemu is working as device model in a *may* be needed? I don't undertstand why this patch is necessary and the commit comment isn't really helping me. > domain that is Domain-0, e.g. in driver domain. > > "owner" parameter for qemu_xen_xs_create() function can have special > value XS_PRESERVE_OWNER, which will make specific implementation to > get original owner of an entry and pass it back to > set_permissions() call. > > Please note, that XenStore inherits permissions, so even if entry is > newly created by, it already has the owner set to match owner of entry > at previous level. > > Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
On Wed, 22 Nov 2023, Paul Durrant wrote: > On 21/11/2023 22:10, Volodymyr Babchuk wrote: > > Add option to preserve owner when creating an entry in Xen Store. This > > may be needed in cases when Qemu is working as device model in a > > *may* be needed? > > I don't undertstand why this patch is necessary and the commit comment isn't > really helping me. > > > domain that is Domain-0, e.g. in driver domain. I think Volodymyr meant: domain that is *not* Domain-0 So I am guessing this patch is needed otherwise QEMU will run into permissions errors doing xenstore operations > > "owner" parameter for qemu_xen_xs_create() function can have special > > value XS_PRESERVE_OWNER, which will make specific implementation to > > get original owner of an entry and pass it back to > > set_permissions() call. > > > > Please note, that XenStore inherits permissions, so even if entry is > > newly created by, it already has the owner set to match owner of entry > > at previous level. > > > > Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com> >
© 2016 - 2024 Red Hat, Inc.