Re: [PATCH v2 4/6] xen_pvdev: Do not assume Dom0 when creating a directory

[Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>]

Hi David,

David Woodhouse <dwmw2@xxxxxxxxxxxxx> writes:

> On 23 November 2023 11:54:01 GMT, Volodymyr Babchuk 
> <Volodymyr_Babchuk@xxxxxxxx> wrote:
>>
>>Hi Paul,
>>
>>Paul Durrant <xadimgnik@xxxxxxxxx> writes:
>>
>>> On 23/11/2023 00:07, Volodymyr Babchuk wrote:
>>>> Hi,
>>>> Volodymyr Babchuk <volodymyr_babchuk@xxxxxxxx> writes:
>>>> 
>>>>> Hi Stefano,
>>>>>
>>>>> Stefano Stabellini <sstabellini@xxxxxxxxxx> 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 <oleksandr_tyshchenko@xxxxxxxx>
>>>>>>>>>>>>
>>>>>>>>>>>> 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