Re: [Qemu-devel] [PATCHv6 0/6] block: add native support for NFS

[ronnie sahlberg]

Please try to sync libnfs again.   Autotools is still a bit of a mystery for me.

On Tue, Jan 28, 2014 at 2:16 AM, Stefan Hajnoczi <address@hidden> wrote:
> On Mon, Jan 27, 2014 at 07:28:07PM +0100, Peter Lieven wrote:
>> Am 27.01.2014 17:11, schrieb Stefan Hajnoczi:
>> > On Mon, Jan 13, 2014 at 11:21:52AM +0100, Peter Lieven wrote:
>> >> This adds v6 of the NFS protocol driver + qemu-iotest adjustments.
>> >>
>> >> v5->v6:
>> >>  - use internal qemu function to parse the NFS url [Kevin]
>> >>  - zero pad short reads [Kevin, Paolo]
>> >>  - added qemu-iotests patches for basic nfs protocol support
>> >>
>> >> v4->v5:
>> >>  - disussed with Ronnie and decided to move URL + Paramter parsing to 
>> >> LibNFS.
>> >>    This allows for URL parameter processing directly in LibNFS without 
>> >> altering
>> >>    the qemu NFS block driver. This bumps the version requirement for 
>> >> LibNFS
>> >>    to 1.9.0 though.
>> >>  - added a pointer to the LibNFS readme where additional information about
>> >>    ROOT privilidge requirements can be found as this raised a few 
>> >> concerns.
>> >>  - removed a trailing dot in an error statement [Fam].
>> >>
>> >> v3->v4:
>> >>  - finally added full implementation of bdrv_get_allocated_file_size 
>> >> [Stefan]
>> >>  - removed trailing \n from error statements [Stefan]
>> >>
>> >> v2->v3:
>> >>  - rebased the stefanha/block
>> >>  - use pkg_config to check for libnfs (ignoring cflags which are broken 
>> >> in 1.8.0) [Stefan]
>> >>  - fixed NFSClient declaration [Stefan]
>> >>  - renamed Task variables to task [Stefan]
>> >>  - renamed NFSTask to NFSRPC [Ronnie]
>> >>  - do not update bs->total_sectors in nfs_co_writev [Stefan]
>> >>  - return -ENOMEM on all async call failures [Stefan,Ronnie]
>> >>  - fully implement ftruncate
>> >>  - use util/uri.c for URL parsing [Stefan]
>> >>  - reworked nfs_file_open_common to nfs_client_open which works on 
>> >> NFSClient [Stefan]
>> >>  - added a comment ot the connect message that libnfs support NFSv3 only 
>> >> at the moment.
>> >>  - DID NOT add full implementation of bdrv_get_allocated_file_size because
>> >>    we are not in a coroutine context and I cannot do an async call here.
>> >>    I could do a sync call if there would be a guarantee that no requests
>> >>    are in flight. [Stefan]
>> >>
>> >> v1->v2:
>> >>  - fixed block/Makefile.objs [Ronnie]
>> >>  - do not always register a read handler [Ronnie]
>> >>  - add support for reading beyond EOF [Fam]
>> >>  - fixed struct and paramter naming [Fam]
>> >>  - fixed overlong lines and whitespace errors [Fam]
>> >>  - return return status from libnfs whereever possible [Fam]
>> >>  - added comment why we set allocated_file_size to -ENOTSUP after write 
>> >> [Fam]
>> >>  - avoid segfault when parsing filname [Fam]
>> >>  - remove unused close_bh from NFSClient [Fam]
>> >>  - avoid dividing and mutliplying total_size by BDRV_SECTOR_SIZE in 
>> >> nfs_file_create [Fam]
>> >>
>> >> Peter Lieven (6):
>> >>   block: add native support for NFS
>> >>   qemu-iotests: change _supported_proto to file for various tests
>> >>   qemu-iotests: enable support for NFS protocol
>> >>   qemu-iotests: enable test 016 and 025 to work with NFS protocol
>> >>   qemu-iotests: fix expected output of test 067
>> >>   qemu-iotests: blacklist test 020 for NFS protocol
>> >>
>> >>  MAINTAINERS                  |    5 +
>> >>  block/Makefile.objs          |    1 +
>> >>  block/nfs.c                  |  444 
>> >> ++++++++++++++++++++++++++++++++++++++++++
>> >>  configure                    |   26 +++
>> >>  qapi-schema.json             |    1 +
>> >>  tests/qemu-iotests/013       |    2 +-
>> >>  tests/qemu-iotests/014       |    2 +-
>> >>  tests/qemu-iotests/016       |    2 +-
>> >>  tests/qemu-iotests/018       |    2 +-
>> >>  tests/qemu-iotests/019       |    2 +-
>> >>  tests/qemu-iotests/020       |    7 +-
>> >>  tests/qemu-iotests/023       |    2 +-
>> >>  tests/qemu-iotests/024       |    2 +-
>> >>  tests/qemu-iotests/025       |    2 +-
>> >>  tests/qemu-iotests/026       |    2 +-
>> >>  tests/qemu-iotests/028       |    2 +-
>> >>  tests/qemu-iotests/031       |    2 +-
>> >>  tests/qemu-iotests/034       |    2 +-
>> >>  tests/qemu-iotests/036       |    2 +-
>> >>  tests/qemu-iotests/037       |    2 +-
>> >>  tests/qemu-iotests/038       |    2 +-
>> >>  tests/qemu-iotests/039       |    2 +-
>> >>  tests/qemu-iotests/043       |    2 +-
>> >>  tests/qemu-iotests/046       |    2 +-
>> >>  tests/qemu-iotests/052       |    2 +-
>> >>  tests/qemu-iotests/054       |    2 +-
>> >>  tests/qemu-iotests/059       |    2 +-
>> >>  tests/qemu-iotests/060       |    2 +-
>> >>  tests/qemu-iotests/061       |    2 +-
>> >>  tests/qemu-iotests/063       |    2 +-
>> >>  tests/qemu-iotests/067.out   |    8 +-
>> >>  tests/qemu-iotests/069       |    2 +-
>> >>  tests/qemu-iotests/common    |   22 ++-
>> >>  tests/qemu-iotests/common.rc |    3 +
>> >>  34 files changed, 534 insertions(+), 33 deletions(-)
>> >>  create mode 100644 block/nfs.c
>> > Any update on the qemu-iotests changes discussed in this thread?  The
>> > actual NFS patch looks fine.
>> I would like to leave the rework of patch 5 to the maintainers.
>>
>> Maybe you can merge the rest of the series except for this change in patch 2.
>>
>> diff --git a/tests/qemu-iotests/020 b/tests/qemu-iotests/020
>> index b3c86d8..a42f32f 100755
>> --- a/tests/qemu-iotests/020
>> +++ b/tests/qemu-iotests/020
>> @@ -43,7 +43,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
>>
>>  # Any format supporting backing files
>>  _supported_fmt qcow qcow2 vmdk qed
>> -_supported_proto generic
>> +_supported_proto file
>>  _supported_os Linux
>>
>> I blacklist patch 20 for NFS manually in Patch 6 as Fam noticed.
>
> I cloned libnfs (fb6f315257a8752c3a8b96a71261a5613a6f2069) and found it
> does not install nfsc/libnfs-raw-mount.h.  Any idea what's going on
> here, should the include be dropped from your patch?
>
> qemu_opts_create_nofail() no longer exists (see
> 87ea75d5e135c0527c6a9dbac4317913409f28c7), the following is needed to
> make it compile:
> -    opts = qemu_opts_create_nofail(&runtime_opts);
> +    opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort);
>
> Please send a final cleaned up series.
>
> Thanks,
> Stefan