From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel De Graaf Subject: Re: [PATCH] xenbus: Add support for xenbus backend in stub domain Date: Fri, 13 Jan 2012 10:44:04 -0500 Message-ID: <4F105144.8070004@tycho.nsa.gov> References: <1326411330-7915-1-git-send-email-dgdegra@tycho.nsa.gov> <1326411373-7971-1-git-send-email-dgdegra@tycho.nsa.gov> <4F0FF742020000780006C5CE@nat28.tlf.novell.com> <4F103A62.3000403@tycho.nsa.gov> <4F105DD1020000780006C8A5@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <4F105DD1020000780006C8A5@nat28.tlf.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Jan Beulich Cc: xen-devel@lists.xensource.com List-Id: xen-devel@lists.xenproject.org On 01/13/2012 10:37 AM, Jan Beulich wrote: >>>> On 13.01.12 at 15:06, Daniel De Graaf wrote: >> On 01/13/2012 03:20 AM, Jan Beulich wrote: >>>>>> On 13.01.12 at 00:36, Daniel De Graaf wrote: >>>> --- a/include/xen/xenbus_dev.h >>>> +++ b/include/xen/xenbus_dev.h >>>> @@ -38,4 +38,18 @@ >>>> #define IOCTL_XENBUS_BACKEND_EVTCHN \ >>>> _IOC(_IOC_NONE, 'B', 0, 0) >>>> >>>> +#define IOCTL_XENBUS_ALLOC \ >>>> + _IOC(_IOC_NONE, 'B', 1, sizeof(struct ioctl_xenbus_alloc)) >>>> +struct ioctl_xenbus_alloc { >>>> + /* IN */ >>>> + /* The domain ID (must exist) for xenstore */ >>>> + uint16_t dom; >>>> + uint16_t pad; >>>> + /* OUT */ >>>> + /* The port allocated for xenbus communication */ >>>> + uint32_t port; >>>> + /* Always set to GNTTAB_RESERVED_XENSTORE */ >>>> + uint32_t grant_ref; >>>> +}; >>> >>> As said in my reply to the previous patch version - if the functionality >>> differs, the number *and* name should be different from the legacy >>> implementation's. Otherwise, how should compatible user space code >>> be written? >>> >>> Jan >>> >> >> This implementation has the same functionality as the legacy ioctl, > > It doesn't - none of the "OUT" fields above exist there. Are you looking at the same legacy ioctl as I am? I found it in 2.6.18.hg where the structure was defined as: typedef struct xenbus_alloc { domid_t dom; __u32 port; __u32 grant_ref; } xenbus_alloc_t; The port and grant_ref fields were outputs. I added padding for clarity since domid_t is uint16_t. > >> although it has a different number and is performed on a different file, >> so it is already impossible to make automatically compatible userspace >> code. > > That's not what I was aiming at. The problem you're introducing is > that you name the ioctl IOCTL_XENBUS_ALLOC, identical to what > the legacy code named it. Hence one won't be able to write user > mode code to invoke, depending on runtime determination, either > the old or the new function. > > Jan > >> The structure name was changed to match other ioctl arguments, but >> the contents should be the same as in the legacy ioctl. If changing what >> file the ioctl is performed on justifies changing the ioctl name, then I >> would prefer the simpler interface where the domain ID is passed in as the >> ioctl parameter instead of a structure that only has one useful output. > >