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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list