WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] [PATCH] libxenlight driver

To: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH] libxenlight driver
From: Jim Fehlig <jfehlig@xxxxxxxxxx>
Date: Tue, 22 Feb 2011 20:46:35 -0700
Cc: LibVir <libvir-list@xxxxxxxxxx>, xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxx>, Markus Gross <gross@xxxxxxxxxxxxx>
Delivery-date: Tue, 22 Feb 2011 19:49:56 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <alpine.DEB.2.00.1102181059380.2826@kaball-desktop>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <4D5DCB44.3060909@xxxxxxxxxx> <alpine.DEB.2.00.1102181059380.2826@kaball-desktop>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Thunderbird 2.0.0.24 (X11/20100302)
Stefano Stabellini wrote:
> On Fri, 18 Feb 2011, Jim Fehlig wrote:
>   
>> +static int
>> +libxlMakeDiskList(virDomainDefPtr def, libxl_domain_config *d_config)
>> +{
>> +    virDomainDiskDefPtr *l_disks = def->disks;
>> +    int ndisks = def->ndisks;
>> +    libxl_device_disk *x_disks;
>> +    int i;
>> +
>> +    if (VIR_ALLOC_N(x_disks, ndisks) < 0) {
>> +        virReportOOMError();
>> +        return -1;
>> +    }
>> +
>> +    for (i = 0; i < ndisks; i++) {
>> +        if (l_disks[i]->src &&
>> +             (x_disks[i].physpath = strdup(l_disks[i]->src)) == NULL) {
>> +            virReportOOMError();
>> +            goto error;
>> +        }
>> +
>> +        if (l_disks[i]->dst &&
>> +            (x_disks[i].virtpath = strdup(l_disks[i]->dst)) == NULL) {
>> +            virReportOOMError();
>> +            goto error;
>> +        }
>> +
>> +        if (l_disks[i]->driverName) {
>> +            if (STREQ(l_disks[i]->driverName, "tap") ||
>> +                STREQ(l_disks[i]->driverName, "tap2")) {
>> +                if (l_disks[i]->driverType &&
>> +                    STREQ(l_disks[i]->driverType, "qcow2"))
>> +                    x_disks[i].phystype = PHYSTYPE_QCOW2;
>> +                else if (l_disks[i]->driverType &&
>> +                         STREQ(l_disks[i]->driverType, "aio"))
>> +                    x_disks[i].phystype = PHYSTYPE_AIO;
>> +                else if (l_disks[i]->driverType &&
>> +                         STREQ(l_disks[i]->driverType, "vhd"))
>> +                    x_disks[i].phystype = PHYSTYPE_VHD;
>> +            } else if (STREQ(l_disks[i]->driverName, "file")) {
>> +                x_disks[i].phystype = PHYSTYPE_FILE;
>> +            } else if (STREQ(l_disks[i]->driverName, "phy")) {
>> +                x_disks[i].phystype = PHYSTYPE_PHY;
>> +            }
>> +        } else {
>> +            /* Default to file?? */
>> +            x_disks[i].phystype = PHYSTYPE_FILE;
>> +        }
>>     
>
> few days ago the patch that removes phystype and introduces backend and
> format has been applied, so this code needs an update
>   

Thanks for the review and comments.  I've posted a new version based on
xen-unstable c/s 22940.

>> +static void libxlEventHandler(int watch,
>> +                              int fd,
>> +                              int events,
>> +                              void *data)
>> +{
>> +    libxlDriverPrivatePtr driver = libxl_driver;
>> +    virDomainObjPtr vm = data;
>> +    libxlDomainObjPrivatePtr priv;
>> +    libxl_event event;
>> +    libxl_dominfo info;
>> +
>> +    libxlDriverLock(driver);
>> +    virDomainObjLock(vm);
>> +    libxlDriverUnlock(driver);
>> +
>> +    priv = vm->privateData;
>> +
>> +    memset(&event, 0, sizeof(event));
>> +    memset(&info, 0, sizeof(info));
>> +
>> +    if (priv->waiterFD != fd || priv->eventHdl != watch) {
>> +        virEventRemoveHandle(watch);
>> +        goto cleanup;
>> +    }
>> +
>> +    if (!(events & VIR_EVENT_HANDLE_READABLE)) {
>> +        goto cleanup;
>> +    }
>> +
>> +    if (libxl_get_event(&priv->ctx, &event)) {
>> +        goto cleanup;
>> +    }
>> +
>> +    if (event.type == LIBXL_EVENT_DOMAIN_DEATH) {
>> +        /* libxl_event_get_domain_death_info returns 1 if death
>> +         * event was for this domid */
>> +        if (libxl_event_get_domain_death_info(&priv->ctx,
>> +                                              vm->def->id,
>> +                                              &event,
>> +                                              &info) != 1) {
>> +            goto cleanup;
>> +        }
>> +
>> +        virEventRemoveHandle(watch);
>> +        if (info.shutdown_reason == SHUTDOWN_poweroff) {
>> +            libxl_domain_destroy(&priv->ctx, vm->def->id, 0);
>> +            if (vm->persistent) {
>> +                vm->def->id = -1;
>> +                vm->state = VIR_DOMAIN_SHUTOFF;
>> +            } else {
>> +                libxl_free_waiter(priv->dWaiter);
>> +                VIR_FREE(priv->dWaiter);
>> +                virDomainRemoveInactive(&driver->domains, vm);
>> +                vm = NULL;
>> +            }
>> +        } else if (info.shutdown_reason == SHUTDOWN_reboot) {
>> +            libxl_domain_destroy(&priv->ctx, vm->def->id, 0);
>> +            vm->def->id = -1;
>> +            vm->state = VIR_DOMAIN_SHUTOFF;
>> +            sleep(1);
>> +            libxlVmStart(vm, 0);
>>     
>
> we need to handle at least SHUTDOWN_suspend and SHUTDOWN_crash too
>   

The updated patch handles SHUTDOWN_crash similar to SHUTDOWN_poweroff. 
I've ignored SHUTDOWN_suspend since the driver does not yet implement
domainSave function.  Also notice I've ignored any user-specified
actions on these events, e.g. coredump and restart.  But IMO, these type
of improvements can be added incrementally.  I'd like to get a basic
driver upstream so others can easily contribute.

>   
>> +        } else {
>> +            VIR_INFO("Unhandled shutdown_reason %d", info.shutdown_reason);
>> +        }
>> +    }
>>     
>
> we should call libxl_free_event before returning, otherwise we leak two
> strings
>   

Fixed in updated patch.

[...]

>> +
>> +    libxl_driver->caps->privateDataAllocFunc = libxlDomainObjPrivateAlloc;
>> +    libxl_driver->caps->privateDataFreeFunc = libxlDomainObjPrivateFree;
>>     
>
> If I understand correctly privateDataAllocFunc is called at each domain 
> creation
>   

It is called when a domain is introduced to libvirt, either by defining
a persistent domain (e.g. virsh define dom.xml) or creating a transient
domain (e.g. virsh create dom.xml).  The free func is called when a
domain is removed from libvirt, either by undefining a persistent domain
(e.g. virsh undefine dom-name) or poweroff of a transient domain. 
Starting/stopping a persistent domain does not invoke these functions.

Regards,
Jim


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel

<Prev in Thread] Current Thread [Next in Thread>