On Tue, Mar 15, 2022 at 11:37 PM Akihiko Odaki <akihiko.od...@gmail.com>
wrote:
> On 2022/03/16 5:27, Vladislav Yaroshchuk wrote:
> > Signed-off-by: Vladislav Yaroshchuk <vladislav.yaroshc...@jetbrains.com>
> > ---
> > net/vmnet-host.c | 110 ++++++++++++++++++++++++++++++++++++++++++++---
> > 1 file changed, 104 insertions(+), 6 deletions(-)
> >
> > diff --git a/net/vmnet-host.c b/net/vmnet-host.c
> > index a461d507c5..8f7a638967 100644
> > --- a/net/vmnet-host.c
> > +++ b/net/vmnet-host.c
> > @@ -9,16 +9,114 @@
> > */
> >
> > #include "qemu/osdep.h"
> > +#include "qemu/uuid.h"
> > #include "qapi/qapi-types-net.h"
> > -#include "vmnet_int.h"
> > -#include "clients.h"
> > -#include "qemu/error-report.h"
> > #include "qapi/error.h"
> > +#include "clients.h"
> > +#include "vmnet_int.h"
> >
> > #include <vmnet/vmnet.h>
> >
> > +
> > +static bool validate_options(const Netdev *netdev, Error **errp)
> > +{
> > + const NetdevVmnetHostOptions *options = &(netdev->u.vmnet_host);
> > + QemuUUID uuid;
>
> The variable uuid is used only when defined(MAC_OS_VERSION_11_0) && \
> MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_VERSION_11_0 and may result in a
> compilation warning otherwise. It should be in the #if block as
> network_uuid variable in build_if_desc is in the counterpart.
>
> Also I suggest to unify the names of identifiers. There are
> options->net_uuid, uuid, and network_uuid, but the differences tells
> nothing.
>
> This should be the last thing to be addressed (unless I missed something
> again.) Thank you for persistence (It's v19!). I really appreciate your
> contribution.
>
>
Thank you for your help and persistence in the review
process :)
Fixed bad naming and moved 'QemuUUID net_uuid'
definition into #if block in validate_options in v20.
Best Regards,
Vladislav Yaroshchuk
Regards,
> Akihiko Odaki
>
> > +
> > +#if defined(MAC_OS_VERSION_11_0) && \
> > + MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_VERSION_11_0
> > +
> > + if (options->has_net_uuid &&
> > + qemu_uuid_parse(options->net_uuid, &uuid) < 0) {
> > + error_setg(errp, "Invalid UUID provided in 'net-uuid'");
> > + return false;
> > + }
> > +#else
> > + if (options->has_isolated) {
> > + error_setg(errp,
> > + "vmnet-host.isolated feature is "
> > + "unavailable: outdated vmnet.framework API");
> > + return false;
> > + }
> > +
> > + if (options->has_net_uuid) {
> > + error_setg(errp,
> > + "vmnet-host.net-uuid feature is "
> > + "unavailable: outdated vmnet.framework API");
> > + return false;
> > + }
> > +#endif
> > +
> > + if ((options->has_start_address ||
> > + options->has_end_address ||
> > + options->has_subnet_mask) &&
> > + !(options->has_start_address &&
> > + options->has_end_address &&
> > + options->has_subnet_mask)) {
> > + error_setg(errp,
> > + "'start-address', 'end-address', 'subnet-mask' "
> > + "should be provided together");
> > + return false;
> > + }
> > +
> > + return true;
> > +}
> > +
> > +static xpc_object_t build_if_desc(const Netdev *netdev,
> > + NetClientState *nc)
> > +{
> > + const NetdevVmnetHostOptions *options = &(netdev->u.vmnet_host);
> > + xpc_object_t if_desc = xpc_dictionary_create(NULL, NULL, 0);
> > +
> > + xpc_dictionary_set_uint64(if_desc,
> > + vmnet_operation_mode_key,
> > + VMNET_HOST_MODE);
> > +
> > +#if defined(MAC_OS_VERSION_11_0) && \
> > + MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_VERSION_11_0
> > +
> > + xpc_dictionary_set_bool(if_desc,
> > + vmnet_enable_isolation_key,
> > + options->isolated);
> > +
> > + QemuUUID network_uuid;
> > + if (options->has_net_uuid) {
> > + qemu_uuid_parse(options->net_uuid, &network_uuid);
> > + xpc_dictionary_set_uuid(if_desc,
> > + vmnet_network_identifier_key,
> > + network_uuid.data);
> > + }
> > +#endif
> > +
> > + if (options->has_start_address) {
> > + xpc_dictionary_set_string(if_desc,
> > + vmnet_start_address_key,
> > + options->start_address);
> > + xpc_dictionary_set_string(if_desc,
> > + vmnet_end_address_key,
> > + options->end_address);
> > + xpc_dictionary_set_string(if_desc,
> > + vmnet_subnet_mask_key,
> > + options->subnet_mask);
> > + }
> > +
> > + return if_desc;
> > +}
> > +
> > +static NetClientInfo net_vmnet_host_info = {
> > + .type = NET_CLIENT_DRIVER_VMNET_HOST,
> > + .size = sizeof(VmnetState),
> > + .receive = vmnet_receive_common,
> > + .cleanup = vmnet_cleanup_common,
> > +};
> > +
> > int net_init_vmnet_host(const Netdev *netdev, const char *name,
> > - NetClientState *peer, Error **errp) {
> > - error_setg(errp, "vmnet-host is not implemented yet");
> > - return -1;
> > + NetClientState *peer, Error **errp)
> > +{
> > + NetClientState *nc = qemu_new_net_client(&net_vmnet_host_info,
> > + peer, "vmnet-host", name);
> > + if (!validate_options(netdev, errp)) {
> > + return -1;
> > + }
> > + return vmnet_if_create(nc, build_if_desc(netdev, nc), errp);
> > }
>
>
