Re: [Qemu-devel] [PATCH v3] slirp: Implement RFC2132 TFTP server name

[Fam Zheng <famz@redhat.com>]

On Mon, 08/27 16:36, Fam Zheng wrote:
> This new usernet option can be used to add data for option 66 (tftp
> server name) in the BOOTP reply, which is useful in PXE based automatic
> OS install such as OpenBSD.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
> 
> v3: Add doc to qemu-options.hx. [Thomas]
> 
> v2: - Adjust parameter order and field placement to be closer to other
> tftp options. [Samuel]
>     - Drop 'optional'. [Eric]
> ---
>  net/slirp.c      | 5 ++++-
>  qapi/net.json    | 5 ++++-
>  qemu-options.hx  | 7 ++++++-
>  slirp/bootp.c    | 8 ++++++++
>  slirp/bootp.h    | 1 +
>  slirp/libslirp.h | 4 +++-
>  slirp/slirp.c    | 5 ++++-
>  slirp/slirp.h    | 1 +
>  8 files changed, 31 insertions(+), 5 deletions(-)
> 
> diff --git a/net/slirp.c b/net/slirp.c
> index 1e14318b4d..90ef09b40a 100644
> --- a/net/slirp.c
> +++ b/net/slirp.c
> @@ -158,6 +158,7 @@ static int net_slirp_init(NetClientState *peer, const char *model,
>                            const char *vnameserver, const char *vnameserver6,
>                            const char *smb_export, const char *vsmbserver,
>                            const char **dnssearch, const char *vdomainname,
> +                          const char *tftp_server_name,
>                            Error **errp)
>  {
>      /* default settings according to historic slirp */
> @@ -376,6 +377,7 @@ static int net_slirp_init(NetClientState *peer, const char *model,
>  
>      s->slirp = slirp_init(restricted, ipv4, net, mask, host,
>                            ipv6, ip6_prefix, vprefix6_len, ip6_host,
> +                          tftp_server_name,

Bug. tftp_server_name should go after vhostname.

Self-NACK

Fam

>                            vhostname, tftp_export, bootfile, dhcp,
>                            dns, ip6_dns, dnssearch, vdomainname, s);
>      QTAILQ_INSERT_TAIL(&slirp_stacks, s, entry);
> @@ -966,7 +968,8 @@ int net_init_slirp(const Netdev *netdev, const char *name,
>                           user->ipv6_host, user->hostname, user->tftp,
>                           user->bootfile, user->dhcpstart,
>                           user->dns, user->ipv6_dns, user->smb,
> -                         user->smbserver, dnssearch, user->domainname, errp);
> +                         user->smbserver, dnssearch, user->domainname,
> +                         user->tftp_server_name, errp);
>  
>      while (slirp_configs) {
>          config = slirp_configs;
> diff --git a/qapi/net.json b/qapi/net.json
> index c86f351161..8f99fd911d 100644
> --- a/qapi/net.json
> +++ b/qapi/net.json
> @@ -174,6 +174,8 @@
>  #
>  # @guestfwd: forward guest TCP connections
>  #
> +# @tftp-server-name: RFC2132 "TFTP server name" string (Since 3.1)
> +#
>  # Since: 1.2
>  ##
>  { 'struct': 'NetdevUserOptions',
> @@ -198,7 +200,8 @@
>      '*smb':       'str',
>      '*smbserver': 'str',
>      '*hostfwd':   ['String'],
> -    '*guestfwd':  ['String'] } }
> +    '*guestfwd':  ['String'],
> +    '*tftp-server-name': 'str' } }
>  
>  ##
>  # @NetdevTapOptions:
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 5515dfaba5..6ddabd9b81 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -1885,7 +1885,7 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
>      "         [,ipv6[=on|off]][,ipv6-net=addr[/int]][,ipv6-host=addr]\n"
>      "         [,restrict=on|off][,hostname=host][,dhcpstart=addr]\n"
>      "         [,dns=addr][,ipv6-dns=addr][,dnssearch=domain][,domainname=domain]\n"
> -    "         [,tftp=dir][,bootfile=f][,hostfwd=rule][,guestfwd=rule]"
> +    "         [,tftp=dir][,tftp-server-name][,bootfile=f][,hostfwd=rule][,guestfwd=rule]"
>  #ifndef _WIN32
>                                               "[,smb=dir[,smbserver=addr]]\n"
>  #endif
> @@ -2122,6 +2122,11 @@ server. The files in @var{dir} will be exposed as the root of a TFTP server.
>  The TFTP client on the guest must be configured in binary mode (use the command
>  @code{bin} of the Unix TFTP client).
>  
> +@item tftp-server-name=@var{name}
> +In BOOTP reply, broadcast @var{name} as the "TFTP server name" (RFC2132 option
> +66). This can be used to advise the guest to load boot files or configurations
> +from a different server than the host address.
> +
>  @item bootfile=@var{file}
>  When using the user mode network stack, broadcast @var{file} as the BOOTP
>  filename. In conjunction with @option{tftp}, this can be used to network boot
> diff --git a/slirp/bootp.c b/slirp/bootp.c
> index 9e7b53ba94..ed2cf3b96e 100644
> --- a/slirp/bootp.c
> +++ b/slirp/bootp.c
> @@ -306,6 +306,14 @@ static void bootp_reply(Slirp *slirp, const struct bootp_t *bp)
>              q += val;
>          }
>  
> +        if (slirp->tftp_server_name) {
> +            val = strlen(slirp->tftp_server_name);
> +            *q++ = RFC2132_TFTP_SERVER_NAME;
> +            *q++ = val;
> +            memcpy(q, slirp->tftp_server_name, val);
> +            q += val;
> +        }
> +
>          if (slirp->vdnssearch) {
>              size_t spaceleft = sizeof(rbp->bp_vend) - (q - rbp->bp_vend);
>              val = slirp->vdnssearch_len;
> diff --git a/slirp/bootp.h b/slirp/bootp.h
> index 394525733e..4043489835 100644
> --- a/slirp/bootp.h
> +++ b/slirp/bootp.h
> @@ -70,6 +70,7 @@
>  #define RFC2132_MAX_SIZE	57
>  #define RFC2132_RENEWAL_TIME    58
>  #define RFC2132_REBIND_TIME     59
> +#define RFC2132_TFTP_SERVER_NAME 66
>  
>  #define DHCPDISCOVER		1
>  #define DHCPOFFER		2
> diff --git a/slirp/libslirp.h b/slirp/libslirp.h
> index 740408a96e..0e84ab9b52 100644
> --- a/slirp/libslirp.h
> +++ b/slirp/libslirp.h
> @@ -13,10 +13,12 @@ Slirp *slirp_init(int restricted, bool in_enabled, struct in_addr vnetwork,
>                    bool in6_enabled,
>                    struct in6_addr vprefix_addr6, uint8_t vprefix_len,
>                    struct in6_addr vhost6, const char *vhostname,
> +                  const char *tftp_server_name,
>                    const char *tftp_path, const char *bootfile,
>                    struct in_addr vdhcp_start, struct in_addr vnameserver,
>                    struct in6_addr vnameserver6, const char **vdnssearch,
> -                  const char *vdomainname, void *opaque);
> +                  const char *vdomainname,
> +                  void *opaque);
>  void slirp_cleanup(Slirp *slirp);
>  
>  void slirp_pollfds_fill(GArray *pollfds, uint32_t *timeout);
> diff --git a/slirp/slirp.c b/slirp/slirp.c
> index 5c3bd6163f..4073b0174f 100644
> --- a/slirp/slirp.c
> +++ b/slirp/slirp.c
> @@ -283,10 +283,12 @@ Slirp *slirp_init(int restricted, bool in_enabled, struct in_addr vnetwork,
>                    bool in6_enabled,
>                    struct in6_addr vprefix_addr6, uint8_t vprefix_len,
>                    struct in6_addr vhost6, const char *vhostname,
> +                  const char *tftp_server_name,
>                    const char *tftp_path, const char *bootfile,
>                    struct in_addr vdhcp_start, struct in_addr vnameserver,
>                    struct in6_addr vnameserver6, const char **vdnssearch,
> -                  const char *vdomainname, void *opaque)
> +                  const char *vdomainname,
> +                  void *opaque)
>  {
>      Slirp *slirp = g_malloc0(sizeof(Slirp));
>  
> @@ -321,6 +323,7 @@ Slirp *slirp_init(int restricted, bool in_enabled, struct in_addr vnetwork,
>      slirp->vdhcp_startaddr = vdhcp_start;
>      slirp->vnameserver_addr = vnameserver;
>      slirp->vnameserver_addr6 = vnameserver6;
> +    slirp->tftp_server_name = g_strdup(tftp_server_name);
>  
>      if (vdnssearch) {
>          translate_dnssearch(slirp, vdnssearch);
> diff --git a/slirp/slirp.h b/slirp/slirp.h
> index 10b410898a..b80725a0d6 100644
> --- a/slirp/slirp.h
> +++ b/slirp/slirp.h
> @@ -212,6 +212,7 @@ struct Slirp {
>      /* tftp states */
>      char *tftp_prefix;
>      struct tftp_session tftp_sessions[TFTP_SESSIONS_MAX];
> +    char *tftp_server_name;
>  
>      ArpTable arp_table;
>      NdpTable ndp_table;
> -- 
> 2.17.1
> 
>