[libvirt] [PATCH] Drop dependency on pm-is-supported
Cole Robinson
crobinso at redhat.com
Mon Mar 31 14:35:26 UTC 2014
On 03/28/2014 04:07 PM, Eric Blake wrote:
> On 03/28/2014 10:32 AM, Cédric Bosdonnat wrote:
>> From: Cédric Bosdonnat <cedric.bosdonnat at free.fr>
>>
>> pm-is-supported is the only thing needed in pm-utils, better get rid of
>> it since systemd is heavily used for libvirt.
>> ---
>> src/util/virnodesuspend.c | 34 ++++++++++++++++++++--------------
>> 1 file changed, 20 insertions(+), 14 deletions(-)
>
> You also need to modify libvirt.spec.in to drop the dependency.
>
>>
>>
>> + if (virFileReadAll("/sys/power/state", 1024, &buf) < 0)
>> + goto cleanup;
>> +
>> + states = virStringSplit(buf, " ", 0);
>> +
>> + canSuspend = (virStringArrayHasString(states, "mem") ||
>> + virStringArrayHasString(states, "standby"));
>> + canHibernate = virStringArrayHasString(states, "disk");
>
> pm-is-supported checks a bit more than what your replacement checks.
>
> For suspend, it declares yes if any of these succeed:
> grep -q mem /sys/power/state
> [ -c /dev/pmu ] && pm-pmu --check
> grep -q standby /sys/power/state
>
> For hibernate, it requires that BOTH of these succeed:
> [ -f /sys/power/disk ]
> grep -q disk /sys/power/state
>
> For hybrid, it requires that all three succeed:
> [ -f /sys/power/disk ] && \
> grep -q disk /sys/power/state && \
> grep -q suspend /sys/power/disk
>
> as well as having fallback code to fake a hybrid sleep by joining the
> other two states.
>
>
>> +
>> switch (target) {
>> case VIR_NODE_SUSPEND_TARGET_MEM:
>> - cmd = virCommandNewArgList("pm-is-supported", "--suspend", NULL);
>> + *supported = canSuspend;
>> break;
>> case VIR_NODE_SUSPEND_TARGET_DISK:
>> - cmd = virCommandNewArgList("pm-is-supported", "--hibernate", NULL);
>> + *supported = canHibernate;
>> break;
>> case VIR_NODE_SUSPEND_TARGET_HYBRID:
>> - cmd = virCommandNewArgList("pm-is-supported", "--suspend-hybrid", NULL);
>> + *supported = canSuspend && canHibernate;
>
> I'm not sure if your simpler checks will cause us to declare an action
> unsupported on systems where it was previously declared supported by
> pm-is-supported. I think the idea makes sense, but I'd like a second
> opinion that we aren't hurting ourselves by doing fewer checks than what
> we are replacing.
FWIW there's a fedora bug about dropping the libvirt dep:
https://bugzilla.redhat.com/show_bug.cgi?id=919390
And this:
https://www.redhat.com/archives/libvir-list/2013-August/msg00212.html
Which claims there's a logind replacement API. Since we aim to build on RHEL5
(where logind/systemd isn't available), maybe we should preserve this old
implementation and add a new one using the new APIs? Sufficiently new distros
just pass the needed configure flag in their build system.
- Cole
More information about the libvir-list
mailing list