[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt PATCH 1/9] Jailhouse driver: first commit with skeleton code



On Thu, Sep 17, 2020 at 04:23:51PM +0100, Daniel P. Berrangé wrote:
> From: Prakhar Bansal <prakharbansal0910 gmail com>
> 
> ---
>  include/libvirt/virterror.h          |   2 +-
>  libvirt.spec.in                      |   7 +
>  m4/virt-driver-jailhouse.m4          |  42 +++++
>  meson.build                          |   4 +
>  meson_options.txt                    |   1 +
>  src/conf/domain_conf.c               |   1 +
>  src/conf/domain_conf.h               |   1 +
>  src/jailhouse/Makefile.inc.am        |  21 +++
>  src/jailhouse/jailhouse_driver.c     | 219 +++++++++++++++++++++++++++
>  src/jailhouse/jailhouse_driver.h     |  23 +++
>  src/jailhouse/libvirtd_jailhouse.aug |  43 ++++++
>  src/jailhouse/meson.build            |  48 ++++++
>  src/libvirt.c                        |  10 ++
>  src/meson.build                      |   1 +
>  src/qemu/qemu_command.c              |   1 +
>  src/util/virerror.c                  |   1 +
>  16 files changed, 424 insertions(+), 1 deletion(-)
>  create mode 100644 m4/virt-driver-jailhouse.m4
>  create mode 100644 src/jailhouse/Makefile.inc.am
>  create mode 100644 src/jailhouse/jailhouse_driver.c
>  create mode 100644 src/jailhouse/jailhouse_driver.h
>  create mode 100644 src/jailhouse/libvirtd_jailhouse.aug
>  create mode 100644 src/jailhouse/meson.build
> 
> diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h
> index 0f1c32283d..97f2ac16d8 100644
> --- a/include/libvirt/virterror.h
> +++ b/include/libvirt/virterror.h
> @@ -136,7 +136,7 @@ typedef enum {
>  
>      VIR_FROM_TPM = 70,          /* Error from TPM */
>      VIR_FROM_BPF = 71,          /* Error from BPF code */
> -
> +    VIR_FROM_JAILHOUSE = 72,    /* Error from Jailhouse driver */
>  # ifdef VIR_ENUM_SENTINELS
>      VIR_ERR_DOMAIN_LAST
>  # endif
> diff --git a/libvirt.spec.in b/libvirt.spec.in
> index 62b401bd08..ca65063e79 100644
> --- a/libvirt.spec.in
> +++ b/libvirt.spec.in
> @@ -17,6 +17,7 @@
>  %define with_lxc           0%{!?_without_lxc:1}
>  %define with_libxl         0%{!?_without_libxl:1}
>  %define with_vbox          0%{!?_without_vbox:1}
> +%define with_jailhouse     0%{!?_without_jailhouse:1}

The RPM spec targets Fedora and RHEL distros and AFAIK, neither of
these ship jailhouse. So we should fix it to have jailhouse disabled
in the RPM builds.

ie just set  -Ddriver_jailhouse=disabled  in %meson



> diff --git a/src/jailhouse/jailhouse_driver.c b/src/jailhouse/jailhouse_driver.c
> new file mode 100644
> index 0000000000..0175ba771b
> --- /dev/null
> +++ b/src/jailhouse/jailhouse_driver.c
> @@ -0,0 +1,219 @@
> +/*
> + * jailhouse_driver.c: Implementation of driver for Jailhouse hypervisor
> + *
> + * Copyright (C) 2020 Prakhar Bansal
> + *
> + * 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 "jailhouse_driver.h"
> +#include "virtypedparam.h"
> +#include "virerror.h"
> +#include "virstring.h"
> +#include "viralloc.h"
> +#include "domain_conf.h"
> +#include "virfile.h"
> +#include "datatypes.h"
> +#include "vircommand.h"
> +#include <string.h>
> +
> +#define UNUSED(x) (void)(x)

G_GNUC_UNUSED is the way we mark parameters as unused, but....

> +
> +static virDrvOpenStatus
> +jailhouseConnectOpen(virConnectPtr conn,
> +                     virConnectAuthPtr auth,
> +                     virConfPtr conf,
> +                     unsigned int flags)
> +{
> +    UNUSED(conn);
> +    UNUSED(auth);
> +    UNUSED(conf);
> +    UNUSED(flags);
> +    return 0;
> +}
> +
> +static int
> +jailhouseConnectClose(virConnectPtr conn)
> +{
> +    UNUSED(conn);
> +    return 0;
> +}
> +
> +static const char *
> +jailhouseConnectGetType(virConnectPtr conn)
> +{
> +    UNUSED(conn);
> +    return NULL;
> +
> +}
> +
> +static char *
> +jailhouseConnectGetHostname(virConnectPtr conn)
> +{
> +    UNUSED(conn);
> +    return NULL;
> +}
> +
> +static int
> +jailhouseNodeGetInfo(virConnectPtr conn,
> +                     virNodeInfoPtr info)
> +{
> +    UNUSED(conn);
> +    UNUSED(info);
> +    return -1;
> +}
> +
> +static int
> +jailhouseConnectListDomains(virConnectPtr conn,
> +                            int *ids,
> +                            int maxids)
> +{
> +    UNUSED(conn);
> +    UNUSED(ids);
> +    UNUSED(maxids);
> +    return -1;
> +}
> +
> +static int
> +jailhouseConnectNumOfDomains(virConnectPtr conn)
> +{
> +    UNUSED(conn);
> +    return -1;
> +}
> +
> +static int
> +jailhouseConnectListAllDomains(virConnectPtr conn,
> +                               virDomainPtr **domain,
> +                               unsigned int flags)
> +{
> +    UNUSED(conn);
> +    UNUSED(domain);
> +    UNUSED(flags);
> +    return -1;
> +}
> +
> +static virDomainPtr
> +jailhouseDomainLookupByID(virConnectPtr conn,
> +                          int id)
> +{
> +    UNUSED(conn);
> +    UNUSED(id);
> +    return NULL;
> +}
> +
> +static virDomainPtr
> +jailhouseDomainLookupByName(virConnectPtr conn,
> +                            const char *name)
> +{
> +    UNUSED(conn);
> +    UNUSED(name);
> +    return NULL;
> +}
> +
> +static virDomainPtr
> +jailhouseDomainLookupByUUID(virConnectPtr conn,
> +                            const unsigned char *uuid)
> +{
> +    UNUSED(conn);
> +    UNUSED(uuid);
> +    return NULL;
> +}
> +
> +static int
> +jailhouseDomainCreate(virDomainPtr domain)
> +{
> +    UNUSED(domain);
> +    return -1;
> +
> +}
> +
> +static int
> +jailhouseDomainShutdown(virDomainPtr domain)
> +{
> +    UNUSED(domain);
> +    return -1;
> +}
> +
> +
> +static int
> +jailhouseDomainDestroy(virDomainPtr domain)
> +{
> +    UNUSED(domain);
> +    return -1;
> +}
> +
> +static int
> +jailhouseDomainGetInfo(virDomainPtr domain,
> +                       virDomainInfoPtr info)
> +{
> +    UNUSED(domain);
> +    UNUSED(info);
> +    return -1;
> +}
> +
> +static int
> +jailhouseDomainGetState(virDomainPtr domain,
> +                        int *state,
> +                        int *reason,
> +                        unsigned int flags)
> +{
> +    UNUSED(domain);
> +    UNUSED(state);
> +    UNUSED(reason);
> +    UNUSED(flags);
> +    return -1;
> +}
> +
> +static char *
> +jailhouseDomainGetXMLDesc(virDomainPtr domain,
> +                          unsigned int flags)
> +{
> +    UNUSED(domain);
> +    UNUSED(flags);
> +    return NULL;
> +}

Just remove all of these stubs except for Open/Close.

The upper level driver entry points report errors correctly
when a callback is simply not registered.

> +
> +static virHypervisorDriver jailhouseHypervisorDriver = {
> +    .name = "JAILHOUSE",
> +    .connectOpen = jailhouseConnectOpen, /* 6.3.0 */
> +    .connectClose = jailhouseConnectClose, /* 6.3.0 */
> +    .connectListDomains = jailhouseConnectListDomains, /* 6.3.0 */
> +    .connectNumOfDomains = jailhouseConnectNumOfDomains, /* 6.3.0 */
> +    .connectListAllDomains = jailhouseConnectListAllDomains, /* 6.3.0 */
> +    .domainLookupByID = jailhouseDomainLookupByID, /* 6.3.0 */
> +    .domainLookupByUUID = jailhouseDomainLookupByUUID, /* 6.3.0 */
> +    .domainLookupByName = jailhouseDomainLookupByName, /* 6.3.0 */
> +    .domainGetXMLDesc = jailhouseDomainGetXMLDesc, /* 6.3.0 */
> +    .domainCreate = jailhouseDomainCreate, /* 6.3.0 */
> +    .connectGetType = jailhouseConnectGetType, /* 6.3.0 */
> +    .connectGetHostname = jailhouseConnectGetHostname, /* 6.3.0 */
> +    .nodeGetInfo = jailhouseNodeGetInfo, /* 6.3.0 */
> +    .domainShutdown = jailhouseDomainShutdown, /* 6.3.0 */
> +    .domainDestroy = jailhouseDomainDestroy, /* 6.3.0 */
> +    .domainGetInfo = jailhouseDomainGetInfo, /* 6.3.0 */
> +    .domainGetState = jailhouseDomainGetState, /* 6.3.0 */
> +};
> +
> +static virConnectDriver jailhouseConnectDriver = {
> +    .hypervisorDriver = &jailhouseHypervisorDriver,
> +};
> +
> +int
> +jailhouseRegister(void)
> +{
> +    return virRegisterConnectDriver(&jailhouseConnectDriver, false);
> +}

> diff --git a/src/jailhouse/libvirtd_jailhouse.aug b/src/jailhouse/libvirtd_jailhouse.aug
> new file mode 100644
> index 0000000000..96a186eae2
> --- /dev/null
> +++ b/src/jailhouse/libvirtd_jailhouse.aug
> @@ -0,0 +1,43 @@
> +(* /etc/libvirt/jailhouse.conf *)
> +
> +module Libvirtd_jailhouse =
> +   autoload xfm
> +
> +   let eol   = del /[ \t]*\n/ "\n"
> +   let value_sep   = del /[ \t]*=[ \t]*/  " = "
> +   let indent = del /[ \t]*/ ""
> +
> +   let array_sep  = del /,[ \t\n]*/ ", "
> +   let array_start = del /\[[ \t\n]*/ "[ "
> +   let array_end = del /\]/ "]"
> +
> +   let str_val = del /\"/ "\"" . store /[^\"]*/ . del /\"/ "\""
> +   let bool_val = store /0|1/
> +   let int_val = store /[0-9]+/
> +   let str_array_element = [ seq "el" . str_val ] . del /[ \t\n]*/ ""
> +   let str_array_val = counter "el" . array_start . ( str_array_element . ( array_sep . str_array_element ) * ) ? . array_end
> +
> +   let str_entry       (kw:string) = [ key kw . value_sep . str_val ]
> +   let bool_entry      (kw:string) = [ key kw . value_sep . bool_val ]
> +   let int_entry       (kw:string) = [ key kw . value_sep . int_val ]
> +   let str_array_entry (kw:string) = [ key kw . value_sep . str_array_val ]
> +
> +   (* Config entry grouped by function - same order as example config *)
> +   let log_entry = bool_entry "log_with_libvirtd"
> +                 | str_entry "security_driver"
> +                 | bool_entry "security_default_confined"
> +                 | bool_entry "security_require_confined"
> +
> +   (* Each entry in the config is one of the following three ... *)
> +   let entry = log_entry
> +   let comment = [ label "#comment" . del /#[ \t]*/ "# " .  store /([^ \t\n][^\n]*)?/ . del /\n/ "\n" ]
> +   let empty = [ label "#empty" . eol ]
> +
> +   let record = indent . entry . eol
> +
> +   let lns = ( record | comment | empty ) *
> +
> +   let filter = incl "/etc/libvirt/jailhouse.conf"
> +              . Util.stdexcl
> +
> +   let xfm = transform lns filter

This should be added in the patch that adds the corresponding config
file

> diff --git a/src/jailhouse/meson.build b/src/jailhouse/meson.build
> new file mode 100644
> index 0000000000..45ceeecca3
> --- /dev/null
> +++ b/src/jailhouse/meson.build
> @@ -0,0 +1,48 @@
> +jailhouse_sources = files(
> +  'jailhouse_driver.c',
> +)
> +
> +driver_source_files += jailhouse_sources
> +stateful_driver_source_files += jailhouse_sources
> +
> +if conf.has('WITH_JAILHOUSE')
> +  jailhouse_driver_impl = static_library(
> +    'virt_driver_jailhouse_impl',
> +    [
> +      jailhouse_sources,
> +    ],
> +    dependencies: [
> +      access_dep,
> +      src_dep,
> +    ],
> +    include_directories: [
> +      conf_inc_dir,
> +      hypervisor_inc_dir,
> +    ],
> +  )
> +
> +  virt_modules += {
> +    'name': 'virt_driver_jailhouse',
> +    'link_whole': [
> +      jailhouse_driver_impl,
> +    ],
> +    'link_args': [
> +      libvirt_no_undefined,
> +    ],
> +  }
> +
> +  virt_daemons += {
> +    'name': 'virtjailhoused',
> +    'c_args': [
> +      '-DDAEMON_NAME="virtjailhoused"',
> +      '-DMODULE_NAME="jailhouse"'
> +    ],
> +  }
> +
> +  virt_conf_files += files('jailhouse.conf')

The jailhouse.conf file doesn't exist in this patch, so this should be
added later in the series.  Essentially each patch is expected to
successfully build and install, so you can't add build rules until
the file is added.

> +  virt_aug_files += files('libvirtd_jailhouse.aug')

This should be moved too.

> +
> +  virt_daemon_confs += {
> +    'name': 'virtjailhoused',
> +  }
> +endif



Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]