[PATCH v6 1/5] server: Allow volume information queries to be asynchronous

[Erich E. Hoover]

Overall, this patchset has two objectives:
1) remove the volume information duplication in
dlls/kernelbase/volume.c in preference to the version in
dlls/mountmgr.sys/device.c
2) set the stage for fixing the advertisement of reparse point support
( https://github.com/wine-staging/wine-staging/blob/master/patches/ntdll-Junct...
)

Per Zeb's suggestion, these patches are now organized as follows:
0001: allow volume information queries to be asynchronous
0002: allow NtQueryVolumeInformationFile to make async volume
information queries
0003: implement volume information queries for device files
0004: hook up volume information queries for mountmgr
0005: reimplement GetVolumeInformation on top of GetVolumeInformationByHandle

===

This specific patch allows the volume information queries to be
asynchronous on the server side.

v6: No change
v5: No change
v4: Split content from server/device.c and
dlls/ntoskrnl.exe/ntoskrnl.c and moved to patch 4
v3: No change
v2: Split

Best,
Erich

[Zebediah Figura (she/her)]

Sorry I didn't notice these before, but...

On 2/6/21 12:24 PM, Erich E. Hoover wrote:
> From: "Erich E. Hoover" 
> Subject: [PATCH v6 1/5] server: Allow volume information queries to be asynchronous
> Message-Id: 
> Date: Sat, 6 Feb 2021 11:24:44 -0700
>
> Overall, this patchset has two objectives:
> - remove the volume information duplication in
>
> dlls/kernelbase/volume.c in preference to the version in
> dlls/mountmgr.sys/device.c
> 2) set the stage for fixing the advertisement of reparse point support
> ( https://github.com/wine-staging/wine-staging/blob/master/patches/ntdll-Junct...
> )
>
> Per Zeb's suggestion, these patches are now organized as follows:
> 0001: allow volume information queries to be asynchronous
> 0002: allow NtQueryVolumeInformationFile to make async volume
> information queries
> 0003: implement volume information queries for device files
> 0004: hook up volume information queries for mountmgr
> 0005: reimplement GetVolumeInformation on top of GetVolumeInformationByHandle
>
> ===
>
> This specific patch allows the volume information queries to be
> asynchronous on the server side.
>
> v6: No change
> v5: No change
> v4: Split content from server/device.c and
> dlls/ntoskrnl.exe/ntoskrnl.c and moved to patch 4
> v3: No change
> v2: Split
>
> Best,
> Erich
>
> From 01708bb64a4b20c8deba0b0a86b7e3a274090bd4 Mon Sep 17 00:00:00 2001
> From: "Erich E. Hoover" 
> Date: Sat, 23 May 2020 21:39:41 -0600
> Subject: server: Allow volume information queries to be asynchronous.
> Signed-off-by: Erich E. Hoover
>
> server/fd.c         | 15 ++++++++++-----
>  server/file.h       |  4 ++--
>  server/named_pipe.c |  5 +++--
>  server/protocol.def |  2 ++
>  4 files changed, 17 insertions(+), 9 deletions(-)
>
> diff --git a/server/fd.c b/server/fd.c
> index 848203f31ec..ee3cb458d35 100644
> --- a/server/fd.c
> +++ b/server/fd.c
> @@ -2314,9 +2314,10 @@ void default_fd_get_file_info( struct fd *fd, obj_handle_t handle, unsigned int
>  }
>
> /* default get_volume_info() routine */
> -void no_fd_get_volume_info( struct fd *fd, unsigned int info_class )
> +int no_fd_get_volume_info( struct fd *fd, struct async *async, unsigned int info_class )
>  {
>      set_error( STATUS_OBJECT_TYPE_MISMATCH );
> - return 0;
>
> }
>
> /* default ioctl() routine */
> @@ -2608,13 +2609,17 @@ DECL_HANDLER(get_file_info)
>  /* query volume info */
>  DECL_HANDLER(get_volume_info)
>  {
> - struct fd *fd = get_handle_fd_obj( current->process, req->handle, 0 );
> - struct fd *fd = get_handle_fd_obj( current->process, req->async.handle, 0 );
> - struct async *async;
> - if (fd)
> - if (!fd) return;
> -
> - if ((async = create_request_async( fd, fd->comp_flags, &req->async )))
> {
> -
>    fd->fd_ops->get_volume_info( fd, req->info_class );
> -
>    release_object( fd );
> -
>    reply->wait    = async_handoff( async, fd->fd_ops->get_volume_info( fd, async, req->info_class ), NULL, 0 );

Shouldn't we pass 1 to force_blocking here? From the client's 
perspective I believe this call is always synchronous.

(Also, weird spacing before = in that line...)
> -
>    release_object( async );
> }
> - release_object( fd );
>
> }
>
> /* open a file object */
> diff --git a/server/file.h b/server/file.h
> index 686bae084c5..a341793d4ab 100644
> --- a/server/file.h
> +++ b/server/file.h
> @@ -65,7 +65,7 @@ struct fd_ops
>      /* query file info */
>      void (*get_file_info)( struct fd *, obj_handle_t, unsigned int );
>      /* query volume info */
> - void (*get_volume_info)( struct fd *, unsigned int );
> - int (*get_volume_info)( struct fd *, struct async *, unsigned int );
> /* perform an ioctl on the file */
> int (*ioctl)(struct fd *fd, ioctl_code_t code, struct async *async );
> /* queue an async operation */
>
> @@ -112,7 +112,7 @@ extern int no_fd_write( struct fd *fd, struct async *async, file_pos_t pos );
>  extern int no_fd_flush( struct fd *fd, struct async *async );
>  extern void no_fd_get_file_info( struct fd *fd, obj_handle_t handle, unsigned int info_class );
>  extern void default_fd_get_file_info( struct fd *fd, obj_handle_t handle, unsigned int info_class );
> -extern void no_fd_get_volume_info( struct fd *fd, unsigned int info_class );
> +extern int no_fd_get_volume_info( struct fd *fd, struct async *async, unsigned int info_class );
>  extern int no_fd_ioctl( struct fd *fd, ioctl_code_t code, struct async *async );
>  extern int default_fd_ioctl( struct fd *fd, ioctl_code_t code, struct async *async );
>  extern void no_fd_queue_async( struct fd *fd, struct async *async, int type, int count );
> diff --git a/server/named_pipe.c b/server/named_pipe.c
> index 2e80ef3b1aa..a3ce9d463f1 100644
> --- a/server/named_pipe.c
> +++ b/server/named_pipe.c
> @@ -146,7 +146,7 @@ static WCHAR *pipe_end_get_full_name( struct object *obj, data_size_t *len );
>  static int pipe_end_read( struct fd *fd, struct async *async, file_pos_t pos );
>  static int pipe_end_write( struct fd *fd, struct async *async_data, file_pos_t pos );
>  static int pipe_end_flush( struct fd *fd, struct async *async );
> -static void pipe_end_get_volume_info( struct fd *fd, unsigned int info_class );
> +static int pipe_end_get_volume_info( struct fd *fd, struct async *async, unsigned int info_class );
>  static void pipe_end_reselect_async( struct fd *fd, struct async_queue *queue );
>  static void pipe_end_get_file_info( struct fd *fd, obj_handle_t handle, unsigned int info_class );
>
> @@ -742,7 +742,7 @@ static WCHAR *pipe_end_get_full_name( struct object *obj, data_size_t *len )
>      return pipe_end->pipe->obj.ops->get_full_name( &pipe_end->pipe->obj, len );
>  }
>
> -static void pipe_end_get_volume_info( struct fd *fd, unsigned int info_class )
> +static int pipe_end_get_volume_info( struct fd *fd, struct async *async, unsigned int info_class )
>  {
>      switch (info_class)
>      {
> @@ -762,6 +762,7 @@ static void pipe_end_get_volume_info( struct fd *fd, unsigned int info_class )
>      default:
>          set_error( STATUS_NOT_IMPLEMENTED );
>      }
> - return 0;
>
> }
>
> static void message_queue_read( struct pipe_end *pipe_end, struct iosb *iosb )
> diff --git a/server/protocol.def b/server/protocol.def
> index 3747a3b63ec..cd7ed35d9a3 100644
> --- a/server/protocol.def
> +++ b/server/protocol.def
> @@ -1400,9 +1400,11 @@ enum server_fd_type
>
> /* Query volume information */
>  @REQ(get_volume_info)
> - async_data_t  async;        /* async I/O parameters */

Another inconsistent spacing nitpick here.
>  obj_handle_t handle;        /* handle to the file */
>  unsigned int info_class;    /* queried information class */
>
> @REPLY
> - obj_handle_t wait;          /* handle to wait on for blocking read */
> VARARG(data,bytes);         /* volume info data */
>
> @END
>
> -- 
> 2.17.1

[Erich E. Hoover]

On Wed, Feb 10, 2021 at 9:15 PM Zebediah Figura (she/her)
 wrote:
> Sorry I didn't notice these before, but...

That's alright, I always appreciate your feedback - it increases the
chances that AJ will be happy ;)

Best,
Erich
> On 2/6/21 12:24 PM, Erich E. Hoover wrote:
> > From: "Erich E. Hoover" 
> > Subject: [PATCH v6 1/5] server: Allow volume information queries to be asynchronous
> > Message-Id: 
> > Date: Sat, 6 Feb 2021 11:24:44 -0700
> > ...
> > - if ((async = create_request_async( fd, fd->comp_flags, &req->async )))
> > {
> > -
> >    fd->fd_ops->get_volume_info( fd, req->info_class );
> > -
> >    release_object( fd );
> > -
> >    reply->wait    = async_handoff( async, fd->fd_ops->get_volume_info( fd, async, req->info_class ), NULL, 0 );
>
> Shouldn't we pass 1 to force_blocking here? From the client's
> perspective I believe this call is always synchronous.

That makes sense to me, I'll go ahead and change it.
> (Also, weird spacing before = in that line...)

Sorry about that, that's from copy-pasting from another instance of
async_handoff and forgetting to delete the alignment spaces.
> > ...
> >  static void message_queue_read( struct pipe_end *pipe_end, struct iosb *iosb )
> > diff --git a/server/protocol.def b/server/protocol.def
> > index 3747a3b63ec..cd7ed35d9a3 100644
> > --- a/server/protocol.def
> > +++ b/server/protocol.def
> > @@ -1400,9 +1400,11 @@ enum server_fd_type
> >
> > /* Query volume information */
> >  @REQ(get_volume_info)
> > - async_data_t  async;        /* async I/O parameters */
>
> Another inconsistent spacing nitpick here.

Consider it taken care of.
> > ...