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

Re: [libvirt PATCH 4/9] Jailhouse driver: Implementation of DomainCreate* callbacks



On Thu, Sep 17, 2020 at 04:23:54PM +0100, Daniel P. Berrangé wrote:
> From: Prakhar Bansal <prakharbansal0910 gmail com>
> 
> Implemented Jailhouse hypervisor APIs for cell
> load/start/shutdown/destroy.
> ---
>  src/jailhouse/jailhouse_api.c    | 100 ++++++++++++--
>  src/jailhouse/jailhouse_api.h    |  29 +++++
>  src/jailhouse/jailhouse_driver.c | 217 +++++++++++++++++++++++++++++--
>  src/jailhouse/jailhouse_driver.h |   8 ++
>  4 files changed, 335 insertions(+), 19 deletions(-)
> 
> diff --git a/src/jailhouse/jailhouse_api.c b/src/jailhouse/jailhouse_api.c
> index cda00b50e7..783903e939 100644
> --- a/src/jailhouse/jailhouse_api.c
> +++ b/src/jailhouse/jailhouse_api.c
> @@ -43,11 +43,14 @@
>  #define JAILHOUSE_CELLS                         "/sys/devices/jailhouse/cells"
>  #define MAX_JAILHOUSE_SYS_CONFIG_FILE_SIZE      1024*1024
>  #define MAX_JAILHOUSE_CELL_CONFIG_FILE_SIZE     1024
> +#define MAX_JAILHOUSE_CELL_IMAGE_FILE_SIZE      64*1024*1024
>  
>  
>  #define JAILHOUSE_ENABLE               _IOW(0, 0, void *)
>  #define JAILHOUSE_DISABLE              _IO(0, 1)
>  #define JAILHOUSE_CELL_CREATE          _IOW(0, 2, virJailhouseCellCreate)
> +#define JAILHOUSE_CELL_LOAD            _IOW(0, 3, virJailhouseCellLoad)
> +#define JAILHOUSE_CELL_START           _IOW(0, 4, virJailhouseCellId)
>  #define JAILHOUSE_CELL_DESTROY         _IOW(0, 5, virJailhouseCellId)
>  
>  #define VIR_FROM_THIS VIR_FROM_JAILHOUSE
> @@ -68,8 +71,6 @@ int cell_match(const struct dirent *dirent);
>  
>  int createCell(const char *conf_file);
>  
> -int destroyCell(virJailhouseCellId cell_id);
> -
>  int getCellInfo(const unsigned int id,
>                  virJailhouseCellInfoPtr * cell_info);
>  
> @@ -254,7 +255,7 @@ readSysfsCellString(const unsigned int id, const char *entry)
>  }
>  
>  int
> -getCellInfo(const unsigned int id, virJailhouseCellInfoPtr * cell_info_ptr)
> +getCellInfo(const unsigned int id, virJailhouseCellInfoPtr *cell_info_ptr)
>  {
>      char *tmp;
>  
> @@ -345,28 +346,105 @@ getJailhouseCellsInfo(void)
>  }
>  
>  int
> -destroyCell(virJailhouseCellId cell_id)
> +loadImagesInCell(virJailhouseCellId cell_id, char **images, int num_images)
> +{
> +   virJailhousePreloadImagePtr image;
> +   virJailhouseCellLoadPtr cell_load;
> +   g_autofree char *buffer = NULL;
> +   unsigned int n;
> +   int len = -1, err = -1;
> +   VIR_AUTOCLOSE fd = -1;
> +
> +
> +   if (VIR_ALLOC(cell_load) < 0) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       "%s",
> +                       _("Insufficient memory for cell load"));
> +        return -1;
> +   }
> +
> +
> +   if (VIR_ALLOC_N(cell_load->image, num_images) < 0) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       "%s",
> +                       _("Insufficient memory for cell load images"));
> +        return -1;
> +   }

No need to check failure - these abort on OOM.

> +
> +   cell_load->id = cell_id;
> +   cell_load->num_preload_images = num_images;
> +
> +   for (n = 0, image = cell_load->image; n < num_images; n++, image++) {
> +        len = virFileReadAll(images[n], MAX_JAILHOUSE_CELL_IMAGE_FILE_SIZE, &buffer);
> +        if (len < 0 || !buffer) {
> +             virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                            _("Failed to read the image file %s"),
> +                            images[n]);
> +             return -1;
> +        }
> +
> +        image->source_address = (unsigned long)buffer;
> +        image->size = len;
> +
> +        // TODO(Prakhar): Add support for target address.
> +        image->target_address = 0;
> +   }
> +
> +   fd = openDev();

Check error here.

> +
> +   err = ioctl(fd, JAILHOUSE_CELL_LOAD, cell_load);
> +   if (err) {
> +       virReportSystemError(errno,
> +                            _("Loading cell images for %d failed"),
> +                            cell_id.id);
> +       return -1;
> +   }
> +
> +   return 0;
> +}


> -destroyJailhouseCells(virJailhouseCellInfoPtr *cell_info_list G_GNUC_UNUSED)
> +destroyCell(virJailhouseCellId cell_id)
>  {
> +    int err = -1;
> +    VIR_AUTOCLOSE fd = -1;
> +
> +    fd = openDev();

Check error

>  
> -    /* Iterate over all cells in cell_info_list and destroy each cell */
> -    // TODO: Not implemented yet.
> +    err = ioctl(fd, JAILHOUSE_CELL_DESTROY, &cell_id);
> +    if (err) {
> +        virReportSystemError(errno,
> +                             _("Destroying cell %d failed"),
> +                             cell_id.id);
> +
> +        return -1;
> +    }


> +typedef struct _virJailhousePreloadImage virJailhousePreloadImage;
> +typedef virJailhousePreloadImage *virJailhousePreloadImagePtr;
> +
> +struct _virJailhousePreloadImage {
> +    __u64 source_address;

uint64_t, etc

> +    __u64 size;
> +    __u64 target_address;
> +    __u64 padding;
> +};
> +
> +typedef struct _virJailhouseCellLoad virJailhouseCellLoad;
> +typedef virJailhouseCellLoad *virJailhouseCellLoadPtr;
> +
> +struct _virJailhouseCellLoad {
> +    struct _virJailhouseCellId id;
> +    __u32 num_preload_images;
> +    __u32 padding;
> +    struct _virJailhousePreloadImage image[];
> +};
> +
> +

>  jailhouseDomainCreate(virDomainPtr domain)
>  {
> -    UNUSED(domain);
> -    return -1;
> +    return jailhouseDomainCreateWithFlags(domain, 0);
> +}
> +
> +static virDomainPtr
> +jailhouseDomainCreateXML(virConnectPtr conn,
> +                         const char *xml,
> +                         unsigned int flags)
> +{
> +    virJailhouseDriverPtr driver = conn->privateData;
> +    virJailhouseCellInfoPtr cell_info;
> +    virDomainPtr dom = NULL;
> +    virDomainDefPtr def = NULL;
> +    virDomainObjPtr cell = NULL;
> +    virDomainDiskDefPtr disk = NULL;
> +    virJailhouseCellId cell_id;
> +    char **images = NULL;
> +    int num_images = 0, i = 0;
> +    unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE;
> +
> +    if (flags & VIR_DOMAIN_START_VALIDATE)
> +        parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE_SCHEMA;
> +
> +    if ((def = virDomainDefParseString(xml, NULL,
> +                                       NULL, parse_flags)) == NULL)
> +        goto cleanup;
> +
> +    if ((cell = virDomainObjListFindByUUID(driver->domains, def->uuid)))
> +        goto cleanup;
> +
> +    if (virDomainCreateXMLEnsureACL(conn, def) < 0)
> +        goto cleanup;
> +
> +    if (!(cell_info = virJailhouseFindCellByName(driver, def->name))) {
> +        virReportError(VIR_ERR_OPERATION_INVALID,
> +                       _("cell info for %s not found"),
> +                       def->name);
> +        goto cleanup;
> +    }
> +
> +    // Assign cell Id to the domain.

Forgot to mention this on previous patches, but we only want C style
comments /* ... */,  never C++ style.

> +    def->id = cell_info->id.id;
> +
> +    if (!(cell = virDomainObjListAdd(driver->domains, def,
> +                                   NULL,
> +                                   VIR_DOMAIN_OBJ_LIST_ADD_LIVE |
> +                                   VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE, NULL)))
> +        goto cleanup;
> +
> +    def = NULL;
> +
> +    if (cell->def->ndisks < 1) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("Domain XML doesn't contain any disk images"));
> +        goto cleanup;
> +    }
> +
> +    if (VIR_ALLOC_N(images, cell->def->ndisks) < 0)
> +        goto cleanup;
> +
> +    for (i = 0; i < cell->def->ndisks; ++i) {
> +        images[i] = NULL;
> +
> +        if (cell->def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_DISK &&
> +            virDomainDiskGetType(cell->def->disks[i]) == VIR_STORAGE_TYPE_FILE) {
> +            disk = cell->def->disks[i];
> +            const char *src = virDomainDiskGetSource(disk);
> +            if (!src) {
> +                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                               _("First file-based harddisk has no source"));
> +                goto cleanup;
> +            }
> +
> +            images[i] = (char *)src;
> +            num_images++;
> +        } else {
> +            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                           _("A Jailhouse doamin(cell) can ONLY have FILE type disks"));
> +            goto cleanup;
> +        }
> +    }


>  static int
>  jailhouseDomainDestroy(virDomainPtr domain)
>  {
> @@ -356,7 +555,9 @@ static virHypervisorDriver jailhouseHypervisorDriver = {
>      .connectListAllDomains = jailhouseConnectListAllDomains,    /* 6.3.0 */
>      .connectGetType = jailhouseConnectGetType,  /* 6.3.0 */
>      .connectGetHostname = jailhouseConnectGetHostname,  /* 6.3.0 */
> -    .domainCreate = jailhouseDomainCreate,      /* 6.3.0 */
> +    .domainCreate = jailhouseDomainCreate,       /*6.3.0 */

Introduced a whitespace bug

> +    .domainCreateWithFlags = jailhouseDomainCreateWithFlags,    /* 6.3.0 */
> +    .domainCreateXML = jailhouseDomainCreateXML, /* 6.3.0 */
>      .domainShutdown = jailhouseDomainShutdown,  /* 6.3.0 */
>      .domainDestroy = jailhouseDomainDestroy,    /* 6.3.0 */
>      .domainGetInfo = jailhouseDomainGetInfo,    /* 6.3.0 */

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]