[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v1 3/7] xen: xenstore: add possibility to preserve owner
On Fri, 2023-11-10 at 20:42 +0000, 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 > 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. > > Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@xxxxxxxx> I like this, although I'd like it more if XenStore itself offered this facility rather than making QEMU do it. Can we make it abundantly clear that XS_PRESERVE_OWNER is a QEMU internal thing? > hw/i386/kvm/xen_xenstore.c | 18 ++++++++++++++++++ > hw/xen/xen-operations.c | 12 ++++++++++++ > include/hw/xen/xen_backend_ops.h | 2 ++ > 3 files changed, 32 insertions(+) > > diff --git a/hw/i386/kvm/xen_xenstore.c b/hw/i386/kvm/xen_xenstore.c > index 660d0b72f9..7b894a9884 100644 > --- a/hw/i386/kvm/xen_xenstore.c > +++ b/hw/i386/kvm/xen_xenstore.c > @@ -1572,6 +1572,24 @@ static bool xs_be_create(struct qemu_xs_handle *h, > xs_transaction_t t, > return false; > } > > + if (owner == XS_PRESERVE_OWNER) { > + GList *perms; > + char letter; > + > + err = xs_impl_get_perms(h->impl, 0, t, path, &perms); > + if (err) { > + errno = err; > + return false; > + } I guess we get away without a race here because it's all internal and we're holding the QEMU iothread mutex? Perhaps assert that? > + if (sscanf(perms->data, "%c%u", &letter, &owner) != 2) { I'd be slightly happier if you used parse_perm() from xenstore_impl.c, but it's static so I suppose that's fair enough. > + errno = EFAULT; > + g_list_free_full(perms, g_free); > + return false; > + } > + g_list_free_full(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..1df59b3c08 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, 0, path, &num); > + if (tmp == NULL) { > + return false; > + } > + perms_list[0].id = tmp[0].id; > + free(tmp); > + } > + Don't see what saves you from someone else changing it at this point on true Xen though. Which is why I'd prefer XenStore to do it natively. > 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..273e414559 100644 > --- a/include/hw/xen/xen_backend_ops.h > +++ b/include/hw/xen/xen_backend_ops.h > @@ -266,6 +266,8 @@ typedef uint32_t xs_transaction_t; > #define XS_PERM_READ 0x01 > #define XS_PERM_WRITE 0x02 > > +#define XS_PRESERVE_OWNER 0xFFFE > + > struct xenstore_backend_ops { > struct qemu_xs_handle *(*open)(void); > void (*close)(struct qemu_xs_handle *h); Attachment:
smime.p7s
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |