Re: [PATCH 8/9] esx: Add implementation for virConnectListAllDomains()

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

 



On 06/05/2012 07:19 AM, Peter Krempa wrote:
> Esx doesn't use the common virDomainObjimplementation so

s/Objimp/Obj imp/

(Hmm, I think you had the same typo in 7/9)

> this patch adds a separate implementation.
> 
> This driver implementation supports only the following filter flags:
> VIR_CONNECT_LIST_DOMAINS_ACTIVE
> VIR_CONNECT_LIST_DOMAINS_INACTIVE
> VIR_CONNECT_LIST_DOMAINS_TRANSIENT
> VIR_CONNECT_LIST_DOMAINS_PERSISTENT
> The latter two of these are irelevant as Esx only supports persistent

s/irelevant/irrelevant/

> domains, so specifying only VIR_CONNECT_LIST_DOMAINS_TRANSIENT results
> into an empty list.

ESX supports domain states (_RUNNING, ...; see
esxVI_VirtualMachinePowerState_ConvertToLibvirt as used in
esxDomainGetInfo), autostart (esxDomainGetAutostart), and snapshots
(esxDomainSnapshotNum), so we should definitely be supporting those
flags.  Also, since there is no managedsave, we could trivially
implement that the same way we implement _TRANSIENT.

> ---
> New in series. UNTESTED!!! (I don't have access to esx, compiles)

I do, and it compiled (but like hyperv, I didn't test the result).

At this point, it might be wise to respin the series, and push the
individual driver pieces as and when they get reviews from someone that
is actually able to test them.


> +
> +        if (VIR_EXPAND_N(doms, ndoms, 1) < 0)
> +            goto no_memory;
> +        doms[count++] = dom;

Same story about considering VIR_RESIZE_N.

Looking forward to v3 of this series.

-- 
Eric Blake   eblake@xxxxxxxxxx    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]