Re: [PATCHv2 6/7] util: create virvsock.c

[Peter Krempa <pkrempa@xxxxxxxxxx>]

On Thu, May 24, 2018 at 12:39:14 +0200, Ján Tomko wrote:
> A file for vsock-related helper functions.
> virVsockSetGuestCid to set an already-known CID,
> virVsockAcquireGuestCid that will use the first available CID
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1291851
> 
> Signed-off-by: Ján Tomko <jtomko@xxxxxxxxxx>
> ---
>  configure.ac             |  8 +++++
>  src/libvirt_private.syms |  5 +++
>  src/util/Makefile.inc.am |  2 ++
>  src/util/virvsock.c      | 89 ++++++++++++++++++++++++++++++++++++++++++++++++
>  src/util/virvsock.h      | 29 ++++++++++++++++
>  5 files changed, 133 insertions(+)
>  create mode 100644 src/util/virvsock.c
>  create mode 100644 src/util/virvsock.h
> 

[...]

> diff --git a/src/util/virvsock.c b/src/util/virvsock.c
> new file mode 100644
> index 0000000000..8a5c88700b
> --- /dev/null
> +++ b/src/util/virvsock.c
> @@ -0,0 +1,89 @@
> +/*
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library.  If not, see
> + * <http://www.gnu.org/licenses/>.
> + *
> + */
> +
> +#include <config.h>
> +
> +#include <sys/ioctl.h>
> +/* #include <fcntl.h> */

Leftover historic includes?

> +
> +#if HAVE_DECL_VHOST_VSOCK_SET_GUEST_CID
> +# include <linux/vhost.h>
> +#endif
> +
> +#include "virvsock.h"
> +
> +#include "virerror.h"
> +#include "virlog.h"
> +
> +
> +#define VIR_FROM_THIS VIR_FROM_NONE
> +
> +VIR_LOG_INIT("util.vsock");
> +
> +#if HAVE_DECL_VHOST_VSOCK_SET_GUEST_CID
> +static int
> +virVsockSetGuestCidQuiet(int fd,
> +                         unsigned int guest_cid)
> +{
> +    uint64_t val = guest_cid;
> +
> +    return ioctl(fd, VHOST_VSOCK_SET_GUEST_CID, &val);
> +}
> +
> +#else
> +static int
> +virVsockSetGuestCidQuiet(int fd ATTRIBUTE_UNUSED,
> +                         unsigned int guest_cid ATTRIBUTE_UNUSED)
> +{
> +    errno = ENOSYS;
> +    return -1;
> +}
> +#endif
> +
> +
> +int
> +virVsockSetGuestCid(int fd,
> +                    unsigned int guest_cid)

Missing docs for the internal public API.

> +{
> +    if (virVsockSetGuestCidQuiet(fd, guest_cid) < 0) {
> +        virReportSystemError(errno, "%s",
> +                             _("failed to set guest cid"));
> +        return -1;
> +    }
> +
> +    return 0;
> +}
> +
> +#define VIR_VSOCK_GUEST_CID_MIN 3
> +
> +int
> +virVsockAcquireGuestCid(int fd,
> +                        unsigned int *guest_cid)

Missing docs for the internal public API.


> +{
> +    unsigned int cid = VIR_VSOCK_GUEST_CID_MIN;
> +
> +    for (; virVsockSetGuestCidQuiet(fd, cid) < 0; cid++) {

It might be worth using an internal static atomic variable as a start of
the loop so that we don't iterate cids which may still be in use.

> +        if (errno != EADDRINUSE) {
> +            virReportSystemError(errno, "%s",
> +                                 _("failed to acquire guest cid"));
> +            return -1;
> +        }
> +    }
> +    *guest_cid = cid;
> +
> +    return 0;
> +}

ACK if you add the docs and remove the commented out include.

Attachment: signature.asc
Description: PGP signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list