[libvirt] [PATCH 06/53] vircgroup: introduce virCgroupV2DetectMounts

Pavel Hrdina posted 53 patches 6 years, 7 months ago
There is a newer version of this series
[libvirt] [PATCH 06/53] vircgroup: introduce virCgroupV2DetectMounts
Posted by Pavel Hrdina 6 years, 7 months ago
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
---
 src/util/vircgroupv2.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c
index 11d9876d36..eaf07397d5 100644
--- a/src/util/vircgroupv2.c
+++ b/src/util/vircgroupv2.c
@@ -167,6 +167,21 @@ virCgroupV2CopyPlacement(virCgroupPtr group,
 }
 
 
+static int
+virCgroupV2DetectMounts(virCgroupPtr group,
+                        const char *mntType,
+                        const char *mntOpts ATTRIBUTE_UNUSED,
+                        const char *mntDir)
+{
+    if (STRNEQ(mntType, "cgroup2"))
+        return 0;
+
+    VIR_FREE(group->unified.mountPoint);
+
+    return VIR_STRDUP(group->unified.mountPoint, mntDir);
+}
+
+
 virCgroupBackend virCgroupV2Backend = {
     .type = VIR_CGROUP_BACKEND_TYPE_V2,
 
@@ -174,6 +189,7 @@ virCgroupBackend virCgroupV2Backend = {
     .validateMachineGroup = virCgroupV2ValidateMachineGroup,
     .copyMounts = virCgroupV2CopyMounts,
     .copyPlacement = virCgroupV2CopyPlacement,
+    .detectMounts = virCgroupV2DetectMounts,
 };
 
 
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 06/53] vircgroup: introduce virCgroupV2DetectMounts
Posted by Michal Privoznik 6 years, 7 months ago
On 10/02/2018 10:43 AM, Pavel Hrdina wrote:
> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> ---
>  src/util/vircgroupv2.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c
> index 11d9876d36..eaf07397d5 100644
> --- a/src/util/vircgroupv2.c
> +++ b/src/util/vircgroupv2.c
> @@ -167,6 +167,21 @@ virCgroupV2CopyPlacement(virCgroupPtr group,
>  }
>  
>  
> +static int
> +virCgroupV2DetectMounts(virCgroupPtr group,
> +                        const char *mntType,
> +                        const char *mntOpts ATTRIBUTE_UNUSED,
> +                        const char *mntDir)
> +{
> +    if (STRNEQ(mntType, "cgroup2"))
> +        return 0;
> +
> +    VIR_FREE(group->unified.mountPoint);
> +
> +    return VIR_STRDUP(group->unified.mountPoint, mntDir);

Looking at virCgroupDetectMounts() maybe we could have a new return
value to stop going through mount table once we've found what we were
looking for? E.g. -1 = an error, 0 not found but continue, 1 found and
break the loop.

Because the way this is written now - this will always pick up the last
cgroup2 mount. I don't think that is a problem right now. Also, probably
if you have more than one mount of cgroup2 then you're in a bigger
trouble anyway. If you decide to work my suggestion in, then it can be
saved as a follow up patch.

> +}
> +
> +
>  virCgroupBackend virCgroupV2Backend = {
>      .type = VIR_CGROUP_BACKEND_TYPE_V2,
>  
> @@ -174,6 +189,7 @@ virCgroupBackend virCgroupV2Backend = {
>      .validateMachineGroup = virCgroupV2ValidateMachineGroup,
>      .copyMounts = virCgroupV2CopyMounts,
>      .copyPlacement = virCgroupV2CopyPlacement,
> +    .detectMounts = virCgroupV2DetectMounts,
>  };
>  
>  
> 


ACK

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 06/53] vircgroup: introduce virCgroupV2DetectMounts
Posted by Pavel Hrdina 6 years, 7 months ago
On Thu, Oct 04, 2018 at 01:18:21PM +0200, Michal Privoznik wrote:
> On 10/02/2018 10:43 AM, Pavel Hrdina wrote:
> > Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> > ---
> >  src/util/vircgroupv2.c | 16 ++++++++++++++++
> >  1 file changed, 16 insertions(+)
> > 
> > diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c
> > index 11d9876d36..eaf07397d5 100644
> > --- a/src/util/vircgroupv2.c
> > +++ b/src/util/vircgroupv2.c
> > @@ -167,6 +167,21 @@ virCgroupV2CopyPlacement(virCgroupPtr group,
> >  }
> >  
> >  
> > +static int
> > +virCgroupV2DetectMounts(virCgroupPtr group,
> > +                        const char *mntType,
> > +                        const char *mntOpts ATTRIBUTE_UNUSED,
> > +                        const char *mntDir)
> > +{
> > +    if (STRNEQ(mntType, "cgroup2"))
> > +        return 0;
> > +
> > +    VIR_FREE(group->unified.mountPoint);
> > +
> > +    return VIR_STRDUP(group->unified.mountPoint, mntDir);
> 
> Looking at virCgroupDetectMounts() maybe we could have a new return
> value to stop going through mount table once we've found what we were
> looking for? E.g. -1 = an error, 0 not found but continue, 1 found and
> break the loop.
> 
> Because the way this is written now - this will always pick up the last
> cgroup2 mount. I don't think that is a problem right now. Also, probably
> if you have more than one mount of cgroup2 then you're in a bigger
> trouble anyway. If you decide to work my suggestion in, then it can be
> saved as a follow up patch.

So I should have probably used the same comment from cgroup v1 backend.

We do it there as well because of bind mounts and we are taking
into account only the last entry from /proc/mounts.

See commit <dacd160d747> for more information.  I don't know whether it
can/will happen for cgroup v2.

> 
> > +}
> > +
> > +
> >  virCgroupBackend virCgroupV2Backend = {
> >      .type = VIR_CGROUP_BACKEND_TYPE_V2,
> >  
> > @@ -174,6 +189,7 @@ virCgroupBackend virCgroupV2Backend = {
> >      .validateMachineGroup = virCgroupV2ValidateMachineGroup,
> >      .copyMounts = virCgroupV2CopyMounts,
> >      .copyPlacement = virCgroupV2CopyPlacement,
> > +    .detectMounts = virCgroupV2DetectMounts,
> >  };
> >  
> >  
> > 
> 
> 
> ACK
> 
> Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list