Re: [PATCH v2 4/6] xen_pvdev: Do not assume Dom0 when creating a directory
Volodymyr Babchuk writes: > Hi, > > Volodymyr Babchuk writes: > >> Hi Stefano, >> >> Stefano Stabellini writes: >> >>> On Wed, 22 Nov 2023, David Woodhouse wrote: On Wed, 2023-11-22 at 15:09 -0800, Stefano Stabellini wrote: > On Wed, 22 Nov 2023, David Woodhouse wrote: > > On Wed, 2023-11-22 at 14:29 -0800, Stefano Stabellini wrote: > > > On Wed, 22 Nov 2023, Paul Durrant wrote: > > > > On 21/11/2023 22:10, Volodymyr Babchuk wrote: > > > > > From: Oleksandr Tyshchenko The -xen-domid argument on the QEMU command line today is the *guest* domain ID, not the domain ID in which QEMU itself is running. Or were you thinking of something different? >>> >>> Ops, you are right and I understand your comment better now. The backend >>> domid is not on the command line but it should be discoverable (on >>> xenstore if I remember right). >> >> Yes, it is just "~/domid". I'll add a function that reads it. > > Just a quick question to QEMU folks: is it better to add a global > variable where we will store own Domain ID or it will be okay to read > domid from Xenstore every time we need it? > > If global variable variant is better, what is proffered place to define > this variable? system/globals.c ? Hmm maybe, I see Xen already has some but the comment: "Global variables that (mostly) should not exist" I think it to dissuade the file growing more than it should. I think generally the best pattern to use if a global can't be avoided is to have a "static global" in the main .c file for the module and then provide a helper access function for other files to read it. That also makes re-factoring easier if things like locking need to be added down the line. We still do have a few true global variables which need "extern" declarations in the headers but if we can avoid adding more that would be good. Of course ideally this sort of data would be wrapped up in QOM structures but I can see the argument for the host domain ID. -- Alex Bennée Virtualisation Tech Lead @ Linaro
Re: [PATCH v2 4/6] xen_pvdev: Do not assume Dom0 when creating a directory
Hi David,
David Woodhouse writes:
> On 23 November 2023 12:17:57 GMT, Volodymyr Babchuk
> wrote:
>>
>>Hi David,
>>
>>David Woodhouse writes:
>>> Which PV backends do you care about? We already have net, block and console
>>> converted.
>>
>>Well, this is all what we need, actually. Even console only will be
>>sufficient, as we are using QEMU to provide virtio-pci backends, so both
>>storage and networking should be provided by virtio. Are you proposing
>>to just drop this patch at all? I believe we can live without it, yes.
>
> Yeah, I think you can drop anything that's only needed for the legacy
> backends. I'm tempted to make a separate config option to compile those out.
Yep, we need this. Because without the patch ("xen_pvdev: Do not assume
Dom0 when creating a directory") I can't run QEMU in the driver domain:
root@spider-domd:~# qemu-system-aarch64 -machine xenpv -m 128M
xen be core: xs_mkdir device-model/0/backends/vkbd: failed
xen be core: xs_mkdir device-model/0/backends/vkbd: failed
xen be core: xs_mkdir device-model/0/backends/9pfs: failed
xen be core: xs_mkdir device-model/0/backends/9pfs: failed
So yeah, we need something like CONFIG_XEN_LEGACY_BACKENDS option if we
don't want to fix xenstore_mkdir()
--
WBR, Volodymyr
Re: [PATCH v2 4/6] xen_pvdev: Do not assume Dom0 when creating a directory
On 23/11/2023 12:27, David Woodhouse wrote: On 23 November 2023 12:17:57 GMT, Volodymyr Babchuk wrote: Hi David, David Woodhouse writes: Which PV backends do you care about? We already have net, block and console converted. Well, this is all what we need, actually. Even console only will be sufficient, as we are using QEMU to provide virtio-pci backends, so both storage and networking should be provided by virtio. Are you proposing to just drop this patch at all? I believe we can live without it, yes. Yeah, I think you can drop anything that's only needed for the legacy backends. I'm tempted to make a separate config option to compile those out. I think that would be a good idea. The other legacy bacckend that we may need to care about is xenfb... not so much the framebuffer itself, but the mouse and keyboard aspects. The XENVKBD and XENHID drivers expose PV mouse and keyboard to Windows instances so it's be nice if we can avoid the backend withering away. Paul
Re: [PATCH v2 4/6] xen_pvdev: Do not assume Dom0 when creating a directory
On 23 November 2023 12:17:57 GMT, Volodymyr Babchuk wrote: > >Hi David, > >David Woodhouse writes: >> Which PV backends do you care about? We already have net, block and console >> converted. > >Well, this is all what we need, actually. Even console only will be >sufficient, as we are using QEMU to provide virtio-pci backends, so both >storage and networking should be provided by virtio. Are you proposing >to just drop this patch at all? I believe we can live without it, yes. Yeah, I think you can drop anything that's only needed for the legacy backends. I'm tempted to make a separate config option to compile those out.
Re: [PATCH v2 4/6] xen_pvdev: Do not assume Dom0 when creating a directory
Hi David, David Woodhouse writes: > On 23 November 2023 11:54:01 GMT, Volodymyr Babchuk > wrote: >> >>Hi Paul, >> >>Paul Durrant writes: >> >>> On 23/11/2023 00:07, Volodymyr Babchuk wrote: Hi, Volodymyr Babchuk writes: > Hi Stefano, > > Stefano Stabellini writes: > >> On Wed, 22 Nov 2023, David Woodhouse wrote: >>> On Wed, 2023-11-22 at 15:09 -0800, Stefano Stabellini wrote: On Wed, 22 Nov 2023, David Woodhouse wrote: > On Wed, 2023-11-22 at 14:29 -0800, Stefano Stabellini wrote: >> On Wed, 22 Nov 2023, Paul Durrant wrote: >>> On 21/11/2023 22:10, Volodymyr Babchuk wrote: From: Oleksandr Tyshchenko Instead of forcing the owner to domid 0, use XS_PRESERVE_OWNER to inherit the owner of the directory. >>> >>> Ah... so that's why the previous patch is there. >>> >>> This is not the right way to fix it. The QEMU Xen support is >>> *assuming* that >>> QEMU is either running in, or emulating, dom0. In the emulation >>> case this is >>> probably fine, but the 'real Xen' case it should be using the >>> correct domid >>> for node creation. I guess this could either be supplied on the >>> command line >>> or discerned by reading the local domain 'domid' node. >> >> yes, it should be passed as command line option to QEMU > > I'm not sure I like the idea of a command line option for something > which QEMU could discover for itself. That's fine too. I meant to say "yes, as far as I know the toolstack passes the domid to QEMU as a command line option today". >>> >>> The -xen-domid argument on the QEMU command line today is the *guest* >>> domain ID, not the domain ID in which QEMU itself is running. >>> >>> Or were you thinking of something different? >> >> Ops, you are right and I understand your comment better now. The backend >> domid is not on the command line but it should be discoverable (on >> xenstore if I remember right). > > Yes, it is just "~/domid". I'll add a function that reads it. Just a quick question to QEMU folks: is it better to add a global variable where we will store own Domain ID or it will be okay to read domid from Xenstore every time we need it? If global variable variant is better, what is proffered place to define this variable? system/globals.c ? >>> >>> Actually... is it possible for QEMU just to use a relative path for >>> the backend nodes? That way it won't need to know it's own domid, will >>> it? >> >>Well, it is possible to use relative path, AFAIK, linux-based backends >>are doing exactly this. But problem is with xenstore_mkdir() function, >>which requires domain id to correctly set owner when it causes call to >>set_permissions(). >> >>As David said, architecturally it will be better to get rid of >>xenstore_mkdir() usage, because it is used by legacy backends only. But >>to do this, someone needs to convert legacy backends to use newer API. I >>have no capacity to do this right now, so I implemented a contained >>solution: >> >>static int xenstore_read_own_domid(unsigned int *domid) >> >>in xen_pvdev.c. I believe, this new function will be removed along with >>whole xen_pvdev.c when there will be no legacy backends left. > > Which PV backends do you care about? We already have net, block and console > converted. Well, this is all what we need, actually. Even console only will be sufficient, as we are using QEMU to provide virtio-pci backends, so both storage and networking should be provided by virtio. Are you proposing to just drop this patch at all? I believe we can live without it, yes. -- WBR, Volodymyr
Re: [PATCH v2 4/6] xen_pvdev: Do not assume Dom0 when creating a directory
On 23 November 2023 11:54:01 GMT, Volodymyr Babchuk wrote: > >Hi Paul, > >Paul Durrant writes: > >> On 23/11/2023 00:07, Volodymyr Babchuk wrote: >>> Hi, >>> Volodymyr Babchuk writes: >>> Hi Stefano, Stefano Stabellini writes: > On Wed, 22 Nov 2023, David Woodhouse wrote: >> On Wed, 2023-11-22 at 15:09 -0800, Stefano Stabellini wrote: >>> On Wed, 22 Nov 2023, David Woodhouse wrote: On Wed, 2023-11-22 at 14:29 -0800, Stefano Stabellini wrote: > On Wed, 22 Nov 2023, Paul Durrant wrote: >> On 21/11/2023 22:10, Volodymyr Babchuk wrote: >>> From: Oleksandr Tyshchenko >>> >>> Instead of forcing the owner to domid 0, use XS_PRESERVE_OWNER to >>> inherit the owner of the directory. >> >> Ah... so that's why the previous patch is there. >> >> This is not the right way to fix it. The QEMU Xen support is >> *assuming* that >> QEMU is either running in, or emulating, dom0. In the emulation case >> this is >> probably fine, but the 'real Xen' case it should be using the >> correct domid >> for node creation. I guess this could either be supplied on the >> command line >> or discerned by reading the local domain 'domid' node. > > yes, it should be passed as command line option to QEMU I'm not sure I like the idea of a command line option for something which QEMU could discover for itself. >>> >>> That's fine too. I meant to say "yes, as far as I know the toolstack >>> passes the domid to QEMU as a command line option today". >> >> The -xen-domid argument on the QEMU command line today is the *guest* >> domain ID, not the domain ID in which QEMU itself is running. >> >> Or were you thinking of something different? > > Ops, you are right and I understand your comment better now. The backend > domid is not on the command line but it should be discoverable (on > xenstore if I remember right). Yes, it is just "~/domid". I'll add a function that reads it. >>> Just a quick question to QEMU folks: is it better to add a global >>> variable where we will store own Domain ID or it will be okay to read >>> domid from Xenstore every time we need it? >>> If global variable variant is better, what is proffered place to >>> define >>> this variable? system/globals.c ? >>> >> >> Actually... is it possible for QEMU just to use a relative path for >> the backend nodes? That way it won't need to know it's own domid, will >> it? > >Well, it is possible to use relative path, AFAIK, linux-based backends >are doing exactly this. But problem is with xenstore_mkdir() function, >which requires domain id to correctly set owner when it causes call to >set_permissions(). > >As David said, architecturally it will be better to get rid of >xenstore_mkdir() usage, because it is used by legacy backends only. But >to do this, someone needs to convert legacy backends to use newer API. I >have no capacity to do this right now, so I implemented a contained >solution: > >static int xenstore_read_own_domid(unsigned int *domid) > >in xen_pvdev.c. I believe, this new function will be removed along with >whole xen_pvdev.c when there will be no legacy backends left. Which PV backends do you care about? We already have net, block and console converted.
Re: [PATCH v2 4/6] xen_pvdev: Do not assume Dom0 when creating a directory
Hi Paul, Paul Durrant writes: > On 23/11/2023 00:07, Volodymyr Babchuk wrote: >> Hi, >> Volodymyr Babchuk writes: >> >>> Hi Stefano, >>> >>> Stefano Stabellini writes: >>> On Wed, 22 Nov 2023, David Woodhouse wrote: > On Wed, 2023-11-22 at 15:09 -0800, Stefano Stabellini wrote: >> On Wed, 22 Nov 2023, David Woodhouse wrote: >>> On Wed, 2023-11-22 at 14:29 -0800, Stefano Stabellini wrote: On Wed, 22 Nov 2023, Paul Durrant wrote: > On 21/11/2023 22:10, Volodymyr Babchuk wrote: >> From: Oleksandr Tyshchenko >> >> Instead of forcing the owner to domid 0, use XS_PRESERVE_OWNER to >> inherit the owner of the directory. > > Ah... so that's why the previous patch is there. > > This is not the right way to fix it. The QEMU Xen support is > *assuming* that > QEMU is either running in, or emulating, dom0. In the emulation case > this is > probably fine, but the 'real Xen' case it should be using the correct > domid > for node creation. I guess this could either be supplied on the > command line > or discerned by reading the local domain 'domid' node. yes, it should be passed as command line option to QEMU >>> >>> I'm not sure I like the idea of a command line option for something >>> which QEMU could discover for itself. >> >> That's fine too. I meant to say "yes, as far as I know the toolstack >> passes the domid to QEMU as a command line option today". > > The -xen-domid argument on the QEMU command line today is the *guest* > domain ID, not the domain ID in which QEMU itself is running. > > Or were you thinking of something different? Ops, you are right and I understand your comment better now. The backend domid is not on the command line but it should be discoverable (on xenstore if I remember right). >>> >>> Yes, it is just "~/domid". I'll add a function that reads it. >> Just a quick question to QEMU folks: is it better to add a global >> variable where we will store own Domain ID or it will be okay to read >> domid from Xenstore every time we need it? >> If global variable variant is better, what is proffered place to >> define >> this variable? system/globals.c ? >> > > Actually... is it possible for QEMU just to use a relative path for > the backend nodes? That way it won't need to know it's own domid, will > it? Well, it is possible to use relative path, AFAIK, linux-based backends are doing exactly this. But problem is with xenstore_mkdir() function, which requires domain id to correctly set owner when it causes call to set_permissions(). As David said, architecturally it will be better to get rid of xenstore_mkdir() usage, because it is used by legacy backends only. But to do this, someone needs to convert legacy backends to use newer API. I have no capacity to do this right now, so I implemented a contained solution: static int xenstore_read_own_domid(unsigned int *domid) in xen_pvdev.c. I believe, this new function will be removed along with whole xen_pvdev.c when there will be no legacy backends left. -- WBR, Volodymyr
Re: [PATCH v2 4/6] xen_pvdev: Do not assume Dom0 when creating a directory
On 23 November 2023 11:43:35 GMT, Volodymyr Babchuk wrote: > >Hi David, > >David Woodhouse writes: > >> [[S/MIME Signed Part:Undecided]] >> On Thu, 2023-11-23 at 09:28 +, Paul Durrant wrote: >>> On 23/11/2023 00:07, Volodymyr Babchuk wrote: >>> > >>> > Hi, >>> > >>> > Volodymyr Babchuk writes: >>> > >>> > > Hi Stefano, >>> > > >>> > > Stefano Stabellini writes: >>> > > >>> > > > On Wed, 22 Nov 2023, David Woodhouse wrote: >>> > > > > On Wed, 2023-11-22 at 15:09 -0800, Stefano Stabellini wrote: >>> > > > > > On Wed, 22 Nov 2023, David Woodhouse wrote: >>> > > > > > > On Wed, 2023-11-22 at 14:29 -0800, Stefano Stabellini wrote: >>> > > > > > > > On Wed, 22 Nov 2023, Paul Durrant wrote: >>> > > > > > > > > On 21/11/2023 22:10, Volodymyr Babchuk wrote: >>> > > > > > > > > > From: Oleksandr Tyshchenko >>> > > > > > > > > > >>> > > > > > > > > > Instead of forcing the owner to domid 0, use >>> > > > > > > > > > XS_PRESERVE_OWNER to >>> > > > > > > > > > inherit the owner of the directory. >>> > > > > > > > > >>> > > > > > > > > Ah... so that's why the previous patch is there. >>> > > > > > > > > >>> > > > > > > > > This is not the right way to fix it. The QEMU Xen support >>> > > > > > > > > is *assuming* that >>> > > > > > > > > QEMU is either running in, or emulating, dom0. In the >>> > > > > > > > > emulation case this is >>> > > > > > > > > probably fine, but the 'real Xen' case it should be using >>> > > > > > > > > the correct domid >>> > > > > > > > > for node creation. I guess this could either be supplied on >>> > > > > > > > > the command line >>> > > > > > > > > or discerned by reading the local domain 'domid' node. >>> > > > > > > > >>> > > > > > > > yes, it should be passed as command line option to QEMU >>> > > > > > > >>> > > > > > > I'm not sure I like the idea of a command line option for >>> > > > > > > something >>> > > > > > > which QEMU could discover for itself. >>> > > > > > >>> > > > > > That's fine too. I meant to say "yes, as far as I know the >>> > > > > > toolstack >>> > > > > > passes the domid to QEMU as a command line option today". >>> > > > > >>> > > > > The -xen-domid argument on the QEMU command line today is the >>> > > > > *guest* >>> > > > > domain ID, not the domain ID in which QEMU itself is running. >>> > > > > >>> > > > > Or were you thinking of something different? >>> > > > >>> > > > Ops, you are right and I understand your comment better now. The >>> > > > backend >>> > > > domid is not on the command line but it should be discoverable (on >>> > > > xenstore if I remember right). >>> > > >>> > > Yes, it is just "~/domid". I'll add a function that reads it. >>> > >>> > Just a quick question to QEMU folks: is it better to add a global >>> > variable where we will store own Domain ID or it will be okay to read >>> > domid from Xenstore every time we need it? >>> > >>> > If global variable variant is better, what is proffered place to define >>> > this variable? system/globals.c ? >>> > >>> >>> Actually... is it possible for QEMU just to use a relative path for the >>> backend nodes? That way it won't need to know it's own domid, will it? >> >> That covers some of the use cases, but it may also need to know its own >> domid for other purposes. Including writing the *absolute* path of the >> backend, to a frontend node? > >Is this case possible? As I understand, QEMU writes frontend nodes only >when it emulates Xen, otherwise this done by Xen toolstack. And in case >of Xen emulation, QEMU always assumes role of Domain-0. No, you can hotplug and unplug devices in QEMU even under real Xen. And if QEMU in a driver domain is given sufficient permission to write to its guest's frontend nodes, it should not need to be in dom0.
Re: [PATCH v2 4/6] xen_pvdev: Do not assume Dom0 when creating a directory
Hi David, David Woodhouse writes: > [[S/MIME Signed Part:Undecided]] > On Thu, 2023-11-23 at 09:28 +, Paul Durrant wrote: >> On 23/11/2023 00:07, Volodymyr Babchuk wrote: >> > >> > Hi, >> > >> > Volodymyr Babchuk writes: >> > >> > > Hi Stefano, >> > > >> > > Stefano Stabellini writes: >> > > >> > > > On Wed, 22 Nov 2023, David Woodhouse wrote: >> > > > > On Wed, 2023-11-22 at 15:09 -0800, Stefano Stabellini wrote: >> > > > > > On Wed, 22 Nov 2023, David Woodhouse wrote: >> > > > > > > On Wed, 2023-11-22 at 14:29 -0800, Stefano Stabellini wrote: >> > > > > > > > On Wed, 22 Nov 2023, Paul Durrant wrote: >> > > > > > > > > On 21/11/2023 22:10, Volodymyr Babchuk wrote: >> > > > > > > > > > From: Oleksandr Tyshchenko >> > > > > > > > > > >> > > > > > > > > > Instead of forcing the owner to domid 0, use >> > > > > > > > > > XS_PRESERVE_OWNER to >> > > > > > > > > > inherit the owner of the directory. >> > > > > > > > > >> > > > > > > > > Ah... so that's why the previous patch is there. >> > > > > > > > > >> > > > > > > > > This is not the right way to fix it. The QEMU Xen support is >> > > > > > > > > *assuming* that >> > > > > > > > > QEMU is either running in, or emulating, dom0. In the >> > > > > > > > > emulation case this is >> > > > > > > > > probably fine, but the 'real Xen' case it should be using >> > > > > > > > > the correct domid >> > > > > > > > > for node creation. I guess this could either be supplied on >> > > > > > > > > the command line >> > > > > > > > > or discerned by reading the local domain 'domid' node. >> > > > > > > > >> > > > > > > > yes, it should be passed as command line option to QEMU >> > > > > > > >> > > > > > > I'm not sure I like the idea of a command line option for >> > > > > > > something >> > > > > > > which QEMU could discover for itself. >> > > > > > >> > > > > > That's fine too. I meant to say "yes, as far as I know the >> > > > > > toolstack >> > > > > > passes the domid to QEMU as a command line option today". >> > > > > >> > > > > The -xen-domid argument on the QEMU command line today is the *guest* >> > > > > domain ID, not the domain ID in which QEMU itself is running. >> > > > > >> > > > > Or were you thinking of something different? >> > > > >> > > > Ops, you are right and I understand your comment better now. The >> > > > backend >> > > > domid is not on the command line but it should be discoverable (on >> > > > xenstore if I remember right). >> > > >> > > Yes, it is just "~/domid". I'll add a function that reads it. >> > >> > Just a quick question to QEMU folks: is it better to add a global >> > variable where we will store own Domain ID or it will be okay to read >> > domid from Xenstore every time we need it? >> > >> > If global variable variant is better, what is proffered place to define >> > this variable? system/globals.c ? >> > >> >> Actually... is it possible for QEMU just to use a relative path for the >> backend nodes? That way it won't need to know it's own domid, will it? > > That covers some of the use cases, but it may also need to know its own > domid for other purposes. Including writing the *absolute* path of the > backend, to a frontend node? Is this case possible? As I understand, QEMU writes frontend nodes only when it emulates Xen, otherwise this done by Xen toolstack. And in case of Xen emulation, QEMU always assumes role of Domain-0. -- WBR, Volodymyr
Re: [PATCH v2 4/6] xen_pvdev: Do not assume Dom0 when creating a directory
On Thu, 2023-11-23 at 09:28 +, Paul Durrant wrote: > On 23/11/2023 00:07, Volodymyr Babchuk wrote: > > > > Hi, > > > > Volodymyr Babchuk writes: > > > > > Hi Stefano, > > > > > > Stefano Stabellini writes: > > > > > > > On Wed, 22 Nov 2023, David Woodhouse wrote: > > > > > On Wed, 2023-11-22 at 15:09 -0800, Stefano Stabellini wrote: > > > > > > On Wed, 22 Nov 2023, David Woodhouse wrote: > > > > > > > On Wed, 2023-11-22 at 14:29 -0800, Stefano Stabellini wrote: > > > > > > > > On Wed, 22 Nov 2023, Paul Durrant wrote: > > > > > > > > > On 21/11/2023 22:10, Volodymyr Babchuk wrote: > > > > > > > > > > From: Oleksandr Tyshchenko > > > > > > > > > > > > > > > > > > > > Instead of forcing the owner to domid 0, use > > > > > > > > > > XS_PRESERVE_OWNER to > > > > > > > > > > inherit the owner of the directory. > > > > > > > > > > > > > > > > > > Ah... so that's why the previous patch is there. > > > > > > > > > > > > > > > > > > This is not the right way to fix it. The QEMU Xen support is > > > > > > > > > *assuming* that > > > > > > > > > QEMU is either running in, or emulating, dom0. In the > > > > > > > > > emulation case this is > > > > > > > > > probably fine, but the 'real Xen' case it should be using the > > > > > > > > > correct domid > > > > > > > > > for node creation. I guess this could either be supplied on > > > > > > > > > the command line > > > > > > > > > or discerned by reading the local domain 'domid' node. > > > > > > > > > > > > > > > > yes, it should be passed as command line option to QEMU > > > > > > > > > > > > > > I'm not sure I like the idea of a command line option for > > > > > > > something > > > > > > > which QEMU could discover for itself. > > > > > > > > > > > > That's fine too. I meant to say "yes, as far as I know the toolstack > > > > > > passes the domid to QEMU as a command line option today". > > > > > > > > > > The -xen-domid argument on the QEMU command line today is the *guest* > > > > > domain ID, not the domain ID in which QEMU itself is running. > > > > > > > > > > Or were you thinking of something different? > > > > > > > > Ops, you are right and I understand your comment better now. The backend > > > > domid is not on the command line but it should be discoverable (on > > > > xenstore if I remember right). > > > > > > Yes, it is just "~/domid". I'll add a function that reads it. > > > > Just a quick question to QEMU folks: is it better to add a global > > variable where we will store own Domain ID or it will be okay to read > > domid from Xenstore every time we need it? > > > > If global variable variant is better, what is proffered place to define > > this variable? system/globals.c ? > > > > Actually... is it possible for QEMU just to use a relative path for the > backend nodes? That way it won't need to know it's own domid, will it? That covers some of the use cases, but it may also need to know its own domid for other purposes. Including writing the *absolute* path of the backend, to a frontend node? smime.p7s Description: S/MIME cryptographic signature
Re: [PATCH v2 4/6] xen_pvdev: Do not assume Dom0 when creating a directory
On 23/11/2023 00:07, Volodymyr Babchuk wrote: Hi, Volodymyr Babchuk writes: Hi Stefano, Stefano Stabellini writes: On Wed, 22 Nov 2023, David Woodhouse wrote: On Wed, 2023-11-22 at 15:09 -0800, Stefano Stabellini wrote: On Wed, 22 Nov 2023, David Woodhouse wrote: On Wed, 2023-11-22 at 14:29 -0800, Stefano Stabellini wrote: On Wed, 22 Nov 2023, Paul Durrant wrote: On 21/11/2023 22:10, Volodymyr Babchuk wrote: From: Oleksandr Tyshchenko Instead of forcing the owner to domid 0, use XS_PRESERVE_OWNER to inherit the owner of the directory. Ah... so that's why the previous patch is there. This is not the right way to fix it. The QEMU Xen support is *assuming* that QEMU is either running in, or emulating, dom0. In the emulation case this is probably fine, but the 'real Xen' case it should be using the correct domid for node creation. I guess this could either be supplied on the command line or discerned by reading the local domain 'domid' node. yes, it should be passed as command line option to QEMU I'm not sure I like the idea of a command line option for something which QEMU could discover for itself. That's fine too. I meant to say "yes, as far as I know the toolstack passes the domid to QEMU as a command line option today". The -xen-domid argument on the QEMU command line today is the *guest* domain ID, not the domain ID in which QEMU itself is running. Or were you thinking of something different? Ops, you are right and I understand your comment better now. The backend domid is not on the command line but it should be discoverable (on xenstore if I remember right). Yes, it is just "~/domid". I'll add a function that reads it. Just a quick question to QEMU folks: is it better to add a global variable where we will store own Domain ID or it will be okay to read domid from Xenstore every time we need it? If global variable variant is better, what is proffered place to define this variable? system/globals.c ? Actually... is it possible for QEMU just to use a relative path for the backend nodes? That way it won't need to know it's own domid, will it? Paul
Re: [PATCH v2 4/6] xen_pvdev: Do not assume Dom0 when creating a directory
Hi, Volodymyr Babchuk writes: > Hi Stefano, > > Stefano Stabellini writes: > >> On Wed, 22 Nov 2023, David Woodhouse wrote: >>> On Wed, 2023-11-22 at 15:09 -0800, Stefano Stabellini wrote: >>> > On Wed, 22 Nov 2023, David Woodhouse wrote: >>> > > On Wed, 2023-11-22 at 14:29 -0800, Stefano Stabellini wrote: >>> > > > On Wed, 22 Nov 2023, Paul Durrant wrote: >>> > > > > On 21/11/2023 22:10, Volodymyr Babchuk wrote: >>> > > > > > From: Oleksandr Tyshchenko >>> > > > > > >>> > > > > > Instead of forcing the owner to domid 0, use XS_PRESERVE_OWNER to >>> > > > > > inherit the owner of the directory. >>> > > > > >>> > > > > Ah... so that's why the previous patch is there. >>> > > > > >>> > > > > This is not the right way to fix it. The QEMU Xen support is >>> > > > > *assuming* that >>> > > > > QEMU is either running in, or emulating, dom0. In the emulation >>> > > > > case this is >>> > > > > probably fine, but the 'real Xen' case it should be using the >>> > > > > correct domid >>> > > > > for node creation. I guess this could either be supplied on the >>> > > > > command line >>> > > > > or discerned by reading the local domain 'domid' node. >>> > > > >>> > > > yes, it should be passed as command line option to QEMU >>> > > >>> > > I'm not sure I like the idea of a command line option for something >>> > > which QEMU could discover for itself. >>> > >>> > That's fine too. I meant to say "yes, as far as I know the toolstack >>> > passes the domid to QEMU as a command line option today". >>> >>> The -xen-domid argument on the QEMU command line today is the *guest* >>> domain ID, not the domain ID in which QEMU itself is running. >>> >>> Or were you thinking of something different? >> >> Ops, you are right and I understand your comment better now. The backend >> domid is not on the command line but it should be discoverable (on >> xenstore if I remember right). > > Yes, it is just "~/domid". I'll add a function that reads it. Just a quick question to QEMU folks: is it better to add a global variable where we will store own Domain ID or it will be okay to read domid from Xenstore every time we need it? If global variable variant is better, what is proffered place to define this variable? system/globals.c ? -- WBR, Volodymyr
Re: [PATCH v2 4/6] xen_pvdev: Do not assume Dom0 when creating a directory
Hi Stefano, Stefano Stabellini writes: > On Wed, 22 Nov 2023, David Woodhouse wrote: >> On Wed, 2023-11-22 at 15:09 -0800, Stefano Stabellini wrote: >> > On Wed, 22 Nov 2023, David Woodhouse wrote: >> > > On Wed, 2023-11-22 at 14:29 -0800, Stefano Stabellini wrote: >> > > > On Wed, 22 Nov 2023, Paul Durrant wrote: >> > > > > On 21/11/2023 22:10, Volodymyr Babchuk wrote: >> > > > > > From: Oleksandr Tyshchenko >> > > > > > >> > > > > > Instead of forcing the owner to domid 0, use XS_PRESERVE_OWNER to >> > > > > > inherit the owner of the directory. >> > > > > >> > > > > Ah... so that's why the previous patch is there. >> > > > > >> > > > > This is not the right way to fix it. The QEMU Xen support is >> > > > > *assuming* that >> > > > > QEMU is either running in, or emulating, dom0. In the emulation case >> > > > > this is >> > > > > probably fine, but the 'real Xen' case it should be using the >> > > > > correct domid >> > > > > for node creation. I guess this could either be supplied on the >> > > > > command line >> > > > > or discerned by reading the local domain 'domid' node. >> > > > >> > > > yes, it should be passed as command line option to QEMU >> > > >> > > I'm not sure I like the idea of a command line option for something >> > > which QEMU could discover for itself. >> > >> > That's fine too. I meant to say "yes, as far as I know the toolstack >> > passes the domid to QEMU as a command line option today". >> >> The -xen-domid argument on the QEMU command line today is the *guest* >> domain ID, not the domain ID in which QEMU itself is running. >> >> Or were you thinking of something different? > > Ops, you are right and I understand your comment better now. The backend > domid is not on the command line but it should be discoverable (on > xenstore if I remember right). Yes, it is just "~/domid". I'll add a function that reads it. -- WBR, Volodymyr
Re: [PATCH v2 4/6] xen_pvdev: Do not assume Dom0 when creating a directory
On Wed, 22 Nov 2023, David Woodhouse wrote: > On Wed, 2023-11-22 at 15:09 -0800, Stefano Stabellini wrote: > > On Wed, 22 Nov 2023, David Woodhouse wrote: > > > On Wed, 2023-11-22 at 14:29 -0800, Stefano Stabellini wrote: > > > > On Wed, 22 Nov 2023, Paul Durrant wrote: > > > > > On 21/11/2023 22:10, Volodymyr Babchuk wrote: > > > > > > From: Oleksandr Tyshchenko > > > > > > > > > > > > Instead of forcing the owner to domid 0, use XS_PRESERVE_OWNER to > > > > > > inherit the owner of the directory. > > > > > > > > > > Ah... so that's why the previous patch is there. > > > > > > > > > > This is not the right way to fix it. The QEMU Xen support is > > > > > *assuming* that > > > > > QEMU is either running in, or emulating, dom0. In the emulation case > > > > > this is > > > > > probably fine, but the 'real Xen' case it should be using the correct > > > > > domid > > > > > for node creation. I guess this could either be supplied on the > > > > > command line > > > > > or discerned by reading the local domain 'domid' node. > > > > > > > > yes, it should be passed as command line option to QEMU > > > > > > I'm not sure I like the idea of a command line option for something > > > which QEMU could discover for itself. > > > > That's fine too. I meant to say "yes, as far as I know the toolstack > > passes the domid to QEMU as a command line option today". > > The -xen-domid argument on the QEMU command line today is the *guest* > domain ID, not the domain ID in which QEMU itself is running. > > Or were you thinking of something different? Ops, you are right and I understand your comment better now. The backend domid is not on the command line but it should be discoverable (on xenstore if I remember right).
Re: [PATCH v2 4/6] xen_pvdev: Do not assume Dom0 when creating a directory
On Wed, 2023-11-22 at 15:09 -0800, Stefano Stabellini wrote: > On Wed, 22 Nov 2023, David Woodhouse wrote: > > On Wed, 2023-11-22 at 14:29 -0800, Stefano Stabellini wrote: > > > On Wed, 22 Nov 2023, Paul Durrant wrote: > > > > On 21/11/2023 22:10, Volodymyr Babchuk wrote: > > > > > From: Oleksandr Tyshchenko > > > > > > > > > > Instead of forcing the owner to domid 0, use XS_PRESERVE_OWNER to > > > > > inherit the owner of the directory. > > > > > > > > Ah... so that's why the previous patch is there. > > > > > > > > This is not the right way to fix it. The QEMU Xen support is *assuming* > > > > that > > > > QEMU is either running in, or emulating, dom0. In the emulation case > > > > this is > > > > probably fine, but the 'real Xen' case it should be using the correct > > > > domid > > > > for node creation. I guess this could either be supplied on the command > > > > line > > > > or discerned by reading the local domain 'domid' node. > > > > > > yes, it should be passed as command line option to QEMU > > > > I'm not sure I like the idea of a command line option for something > > which QEMU could discover for itself. > > That's fine too. I meant to say "yes, as far as I know the toolstack > passes the domid to QEMU as a command line option today". The -xen-domid argument on the QEMU command line today is the *guest* domain ID, not the domain ID in which QEMU itself is running. Or were you thinking of something different? smime.p7s Description: S/MIME cryptographic signature
Re: [PATCH v2 4/6] xen_pvdev: Do not assume Dom0 when creating a directory
On Wed, 22 Nov 2023, David Woodhouse wrote: > On Wed, 2023-11-22 at 14:29 -0800, Stefano Stabellini wrote: > > On Wed, 22 Nov 2023, Paul Durrant wrote: > > > On 21/11/2023 22:10, Volodymyr Babchuk wrote: > > > > From: Oleksandr Tyshchenko > > > > > > > > Instead of forcing the owner to domid 0, use XS_PRESERVE_OWNER to > > > > inherit the owner of the directory. > > > > > > Ah... so that's why the previous patch is there. > > > > > > This is not the right way to fix it. The QEMU Xen support is *assuming* > > > that > > > QEMU is either running in, or emulating, dom0. In the emulation case this > > > is > > > probably fine, but the 'real Xen' case it should be using the correct > > > domid > > > for node creation. I guess this could either be supplied on the command > > > line > > > or discerned by reading the local domain 'domid' node. > > > > yes, it should be passed as command line option to QEMU > > I'm not sure I like the idea of a command line option for something > which QEMU could discover for itself. That's fine too. I meant to say "yes, as far as I know the toolstack passes the domid to QEMU as a command line option today".
Re: [PATCH v2 4/6] xen_pvdev: Do not assume Dom0 when creating a directory
On Wed, 2023-11-22 at 14:29 -0800, Stefano Stabellini wrote: > On Wed, 22 Nov 2023, Paul Durrant wrote: > > On 21/11/2023 22:10, Volodymyr Babchuk wrote: > > > From: Oleksandr Tyshchenko > > > > > > Instead of forcing the owner to domid 0, use XS_PRESERVE_OWNER to > > > inherit the owner of the directory. > > > > Ah... so that's why the previous patch is there. > > > > This is not the right way to fix it. The QEMU Xen support is *assuming* that > > QEMU is either running in, or emulating, dom0. In the emulation case this is > > probably fine, but the 'real Xen' case it should be using the correct domid > > for node creation. I guess this could either be supplied on the command line > > or discerned by reading the local domain 'domid' node. > > yes, it should be passed as command line option to QEMU I'm not sure I like the idea of a command line option for something which QEMU could discover for itself. smime.p7s Description: S/MIME cryptographic signature
Re: [PATCH v2 4/6] xen_pvdev: Do not assume Dom0 when creating a directory
On Wed, 22 Nov 2023, Paul Durrant wrote:
> On 21/11/2023 22:10, Volodymyr Babchuk wrote:
> > From: Oleksandr Tyshchenko
> >
> > Instead of forcing the owner to domid 0, use XS_PRESERVE_OWNER to
> > inherit the owner of the directory.
>
> Ah... so that's why the previous patch is there.
>
> This is not the right way to fix it. The QEMU Xen support is *assuming* that
> QEMU is either running in, or emulating, dom0. In the emulation case this is
> probably fine, but the 'real Xen' case it should be using the correct domid
> for node creation. I guess this could either be supplied on the command line
> or discerned by reading the local domain 'domid' node.
yes, it should be passed as command line option to QEMU
> > Note that for other than Dom0 domain (non toolstack domain) the
> > "driver_domain" property should be set in domain config file for the
> > toolstack to create required directories in advance.
> >
> > Signed-off-by: Oleksandr Tyshchenko
> > Signed-off-by: Volodymyr Babchuk
> > ---
> > hw/xen/xen_pvdev.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/xen/xen_pvdev.c b/hw/xen/xen_pvdev.c
> > index c5ad71e8dc..42bdd4f6c8 100644
> > --- a/hw/xen/xen_pvdev.c
> > +++ b/hw/xen/xen_pvdev.c
> > @@ -60,7 +60,8 @@ void xen_config_cleanup(void)
> > int xenstore_mkdir(char *path, int p)
> > {
> > -if (!qemu_xen_xs_create(xenstore, 0, 0, xen_domid, p, path)) {
> > +if (!qemu_xen_xs_create(xenstore, 0, XS_PRESERVE_OWNER,
> > +xen_domid, p, path)) {
> > xen_pv_printf(NULL, 0, "xs_mkdir %s: failed\n", path);
> > return -1;
> > }
>
Re: [PATCH v2 4/6] xen_pvdev: Do not assume Dom0 when creating a directory
On 21/11/2023 22:10, Volodymyr Babchuk wrote:
From: Oleksandr Tyshchenko
Instead of forcing the owner to domid 0, use XS_PRESERVE_OWNER to
inherit the owner of the directory.
Ah... so that's why the previous patch is there.
This is not the right way to fix it. The QEMU Xen support is *assuming*
that QEMU is either running in, or emulating, dom0. In the emulation
case this is probably fine, but the 'real Xen' case it should be using
the correct domid for node creation. I guess this could either be
supplied on the command line or discerned by reading the local domain
'domid' node.
Note that for other than Dom0 domain (non toolstack domain) the
"driver_domain" property should be set in domain config file for the
toolstack to create required directories in advance.
Signed-off-by: Oleksandr Tyshchenko
Signed-off-by: Volodymyr Babchuk
---
hw/xen/xen_pvdev.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/hw/xen/xen_pvdev.c b/hw/xen/xen_pvdev.c
index c5ad71e8dc..42bdd4f6c8 100644
--- a/hw/xen/xen_pvdev.c
+++ b/hw/xen/xen_pvdev.c
@@ -60,7 +60,8 @@ void xen_config_cleanup(void)
int xenstore_mkdir(char *path, int p)
{
-if (!qemu_xen_xs_create(xenstore, 0, 0, xen_domid, p, path)) {
+if (!qemu_xen_xs_create(xenstore, 0, XS_PRESERVE_OWNER,
+xen_domid, p, path)) {
xen_pv_printf(NULL, 0, "xs_mkdir %s: failed\n", path);
return -1;
}
[PATCH v2 4/6] xen_pvdev: Do not assume Dom0 when creating a directory
From: Oleksandr Tyshchenko
Instead of forcing the owner to domid 0, use XS_PRESERVE_OWNER to
inherit the owner of the directory.
Note that for other than Dom0 domain (non toolstack domain) the
"driver_domain" property should be set in domain config file for the
toolstack to create required directories in advance.
Signed-off-by: Oleksandr Tyshchenko
Signed-off-by: Volodymyr Babchuk
---
hw/xen/xen_pvdev.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/hw/xen/xen_pvdev.c b/hw/xen/xen_pvdev.c
index c5ad71e8dc..42bdd4f6c8 100644
--- a/hw/xen/xen_pvdev.c
+++ b/hw/xen/xen_pvdev.c
@@ -60,7 +60,8 @@ void xen_config_cleanup(void)
int xenstore_mkdir(char *path, int p)
{
-if (!qemu_xen_xs_create(xenstore, 0, 0, xen_domid, p, path)) {
+if (!qemu_xen_xs_create(xenstore, 0, XS_PRESERVE_OWNER,
+xen_domid, p, path)) {
xen_pv_printf(NULL, 0, "xs_mkdir %s: failed\n", path);
return -1;
}
--
2.42.0
