qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] qga: skip bind mounts in fs list
@ 2024-10-02 10:06 Jean-Louis Dupond
  2024-10-09  8:34 ` Jean-Louis Dupond
  0 siblings, 1 reply; 6+ messages in thread
From: Jean-Louis Dupond @ 2024-10-02 10:06 UTC (permalink / raw)
  To: qemu-devel, michael.roth; +Cc: Jean-Louis Dupond

The filesystem list in build_fs_mount_list should skip bind mounts.
This because we end up in locking situations when doing fsFreeze. Like
mentioned in [1] and [2].

Next to that, the build_fs_mount_list call did a fallback via
build_fs_mount_list_from_mtab if mountinfo did not exist.
There it skipped bind mounts, but this is broken for newer OS.
This as mounts does not return the path of the bind mount but the
underlying dev/partition, so S_ISDIR will never return true in
dev_major_minor call.

This patch simply checks the existing devmajor:devminor tuple in the
mounts, and if it already exists, this means we have the same devices
mounted again, a bind mount. So skip this.

Same approach is used in open-vm-tools [3].

[1]: https://gitlab.com/qemu-project/qemu/-/issues/592
[2]: https://gitlab.com/qemu-project/qemu/-/issues/520
[3]: https://github.com/vmware/open-vm-tools/commit/d58847b497e212737007958c945af1df22a8ab58

Signed-off-by: Jean-Louis Dupond <jean-louis@dupond.be>
---
 qga/commands-linux.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/qga/commands-linux.c b/qga/commands-linux.c
index 51d5e3d927..426b040ab8 100644
--- a/qga/commands-linux.c
+++ b/qga/commands-linux.c
@@ -59,6 +59,22 @@ static int dev_major_minor(const char *devpath,
     return -1;
 }
 
+/*
+ * Check if we already have the devmajor:devminor in the mounts
+ * If thats the case return true.
+ */
+static bool dev_exists(FsMountList *mounts, unsigned int devmajor, unsigned int devminor)
+{
+    FsMount *mount;
+
+    QTAILQ_FOREACH(mount, mounts, next) {
+        if (mount->devmajor == devmajor && mount->devminor == devminor) {
+            return true;
+        }
+    }
+    return false;
+}
+
 static bool build_fs_mount_list_from_mtab(FsMountList *mounts, Error **errp)
 {
     struct mntent *ment;
@@ -89,6 +105,10 @@ static bool build_fs_mount_list_from_mtab(FsMountList *mounts, Error **errp)
             /* Skip bind mounts */
             continue;
         }
+        if (dev_exists(mounts, devmajor, devminor)) {
+            /* Skip already existing devices (bind mounts) */
+            continue;
+        }
 
         mount = g_new0(FsMount, 1);
         mount->dirname = g_strdup(ment->mnt_dir);
@@ -172,6 +192,11 @@ bool build_fs_mount_list(FsMountList *mounts, Error **errp)
             }
         }
 
+        if (dev_exists(mounts, devmajor, devminor)) {
+            /* Skip already existing devices (bind mounts) */
+            continue;
+        }
+
         mount = g_new0(FsMount, 1);
         mount->dirname = g_strdup(line + dir_s);
         mount->devtype = g_strdup(dash + type_s);
-- 
2.46.2



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] qga: skip bind mounts in fs list
  2024-10-02 10:06 [PATCH] qga: skip bind mounts in fs list Jean-Louis Dupond
@ 2024-10-09  8:34 ` Jean-Louis Dupond
  2024-10-25 10:06   ` Jean-Louis Dupond
  0 siblings, 1 reply; 6+ messages in thread
From: Jean-Louis Dupond @ 2024-10-09  8:34 UTC (permalink / raw)
  To: qemu-devel, michael.roth, kkostiuk

On 2/10/2024 12:06, Jean-Louis Dupond wrote:
> The filesystem list in build_fs_mount_list should skip bind mounts.
> This because we end up in locking situations when doing fsFreeze. Like
> mentioned in [1] and [2].
>
> Next to that, the build_fs_mount_list call did a fallback via
> build_fs_mount_list_from_mtab if mountinfo did not exist.
> There it skipped bind mounts, but this is broken for newer OS.
> This as mounts does not return the path of the bind mount but the
> underlying dev/partition, so S_ISDIR will never return true in
> dev_major_minor call.
>
> This patch simply checks the existing devmajor:devminor tuple in the
> mounts, and if it already exists, this means we have the same devices
> mounted again, a bind mount. So skip this.
>
> Same approach is used in open-vm-tools [3].
>
> [1]: https://gitlab.com/qemu-project/qemu/-/issues/592
> [2]: https://gitlab.com/qemu-project/qemu/-/issues/520
> [3]: https://github.com/vmware/open-vm-tools/commit/d58847b497e212737007958c945af1df22a8ab58
>
> Signed-off-by: Jean-Louis Dupond <jean-louis@dupond.be>
> ---
>   qga/commands-linux.c | 25 +++++++++++++++++++++++++
>   1 file changed, 25 insertions(+)
>
> diff --git a/qga/commands-linux.c b/qga/commands-linux.c
> index 51d5e3d927..426b040ab8 100644
> --- a/qga/commands-linux.c
> +++ b/qga/commands-linux.c
> @@ -59,6 +59,22 @@ static int dev_major_minor(const char *devpath,
>       return -1;
>   }
>   
> +/*
> + * Check if we already have the devmajor:devminor in the mounts
> + * If thats the case return true.
> + */
> +static bool dev_exists(FsMountList *mounts, unsigned int devmajor, unsigned int devminor)
> +{
> +    FsMount *mount;
> +
> +    QTAILQ_FOREACH(mount, mounts, next) {
> +        if (mount->devmajor == devmajor && mount->devminor == devminor) {
> +            return true;
> +        }
> +    }
> +    return false;
> +}
> +
>   static bool build_fs_mount_list_from_mtab(FsMountList *mounts, Error **errp)
>   {
>       struct mntent *ment;
> @@ -89,6 +105,10 @@ static bool build_fs_mount_list_from_mtab(FsMountList *mounts, Error **errp)
>               /* Skip bind mounts */
>               continue;
>           }
> +        if (dev_exists(mounts, devmajor, devminor)) {
> +            /* Skip already existing devices (bind mounts) */
> +            continue;
> +        }
>   
>           mount = g_new0(FsMount, 1);
>           mount->dirname = g_strdup(ment->mnt_dir);
> @@ -172,6 +192,11 @@ bool build_fs_mount_list(FsMountList *mounts, Error **errp)
>               }
>           }
>   
> +        if (dev_exists(mounts, devmajor, devminor)) {
> +            /* Skip already existing devices (bind mounts) */
> +            continue;
> +        }
> +
>           mount = g_new0(FsMount, 1);
>           mount->dirname = g_strdup(line + dir_s);
>           mount->devtype = g_strdup(dash + type_s);


Ping + add kkostiuk@redhat.com as I missed him in the initial mail.



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] qga: skip bind mounts in fs list
  2024-10-09  8:34 ` Jean-Louis Dupond
@ 2024-10-25 10:06   ` Jean-Louis Dupond
  2024-10-28 10:57     ` Konstantin Kostiuk
  0 siblings, 1 reply; 6+ messages in thread
From: Jean-Louis Dupond @ 2024-10-25 10:06 UTC (permalink / raw)
  To: qemu-devel, michael.roth, kkostiuk

On 9/10/2024 10:34, Jean-Louis Dupond wrote:
> On 2/10/2024 12:06, Jean-Louis Dupond wrote:
>> The filesystem list in build_fs_mount_list should skip bind mounts.
>> This because we end up in locking situations when doing fsFreeze. Like
>> mentioned in [1] and [2].
>>
>> Next to that, the build_fs_mount_list call did a fallback via
>> build_fs_mount_list_from_mtab if mountinfo did not exist.
>> There it skipped bind mounts, but this is broken for newer OS.
>> This as mounts does not return the path of the bind mount but the
>> underlying dev/partition, so S_ISDIR will never return true in
>> dev_major_minor call.
>>
>> This patch simply checks the existing devmajor:devminor tuple in the
>> mounts, and if it already exists, this means we have the same devices
>> mounted again, a bind mount. So skip this.
>>
>> Same approach is used in open-vm-tools [3].
>>
>> [1]: https://gitlab.com/qemu-project/qemu/-/issues/592
>> [2]: https://gitlab.com/qemu-project/qemu/-/issues/520
>> [3]: 
>> https://github.com/vmware/open-vm-tools/commit/d58847b497e212737007958c945af1df22a8ab58
>>
>> Signed-off-by: Jean-Louis Dupond <jean-louis@dupond.be>
>> ---
>>   qga/commands-linux.c | 25 +++++++++++++++++++++++++
>>   1 file changed, 25 insertions(+)
>>
>> diff --git a/qga/commands-linux.c b/qga/commands-linux.c
>> index 51d5e3d927..426b040ab8 100644
>> --- a/qga/commands-linux.c
>> +++ b/qga/commands-linux.c
>> @@ -59,6 +59,22 @@ static int dev_major_minor(const char *devpath,
>>       return -1;
>>   }
>>   +/*
>> + * Check if we already have the devmajor:devminor in the mounts
>> + * If thats the case return true.
>> + */
>> +static bool dev_exists(FsMountList *mounts, unsigned int devmajor, 
>> unsigned int devminor)
>> +{
>> +    FsMount *mount;
>> +
>> +    QTAILQ_FOREACH(mount, mounts, next) {
>> +        if (mount->devmajor == devmajor && mount->devminor == 
>> devminor) {
>> +            return true;
>> +        }
>> +    }
>> +    return false;
>> +}
>> +
>>   static bool build_fs_mount_list_from_mtab(FsMountList *mounts, 
>> Error **errp)
>>   {
>>       struct mntent *ment;
>> @@ -89,6 +105,10 @@ static bool 
>> build_fs_mount_list_from_mtab(FsMountList *mounts, Error **errp)
>>               /* Skip bind mounts */
>>               continue;
>>           }
>> +        if (dev_exists(mounts, devmajor, devminor)) {
>> +            /* Skip already existing devices (bind mounts) */
>> +            continue;
>> +        }
>>             mount = g_new0(FsMount, 1);
>>           mount->dirname = g_strdup(ment->mnt_dir);
>> @@ -172,6 +192,11 @@ bool build_fs_mount_list(FsMountList *mounts, 
>> Error **errp)
>>               }
>>           }
>>   +        if (dev_exists(mounts, devmajor, devminor)) {
>> +            /* Skip already existing devices (bind mounts) */
>> +            continue;
>> +        }
>> +
>>           mount = g_new0(FsMount, 1);
>>           mount->dirname = g_strdup(line + dir_s);
>>           mount->devtype = g_strdup(dash + type_s);
>
>
> Ping + add kkostiuk@redhat.com as I missed him in the initial mail.
>

Any chance on a review or getting it merged?
Think it's a good (of course ;)) improvement.



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] qga: skip bind mounts in fs list
  2024-10-25 10:06   ` Jean-Louis Dupond
@ 2024-10-28 10:57     ` Konstantin Kostiuk
  2024-10-28 12:15       ` Jean-Louis Dupond
  0 siblings, 1 reply; 6+ messages in thread
From: Konstantin Kostiuk @ 2024-10-28 10:57 UTC (permalink / raw)
  To: Jean-Louis Dupond; +Cc: qemu-devel, michael.roth

[-- Attachment #1: Type: text/plain, Size: 3733 bytes --]

Hi Jean-Louis,

Thanks for your patch. I hope next week, I will test and review this patch.

Just a question, did you have a chance to test that this patch fix kernel
crash?

Best Regards,
Konstantin Kostiuk.


On Fri, Oct 25, 2024 at 1:06 PM Jean-Louis Dupond <jean-louis@dupond.be>
wrote:

> On 9/10/2024 10:34, Jean-Louis Dupond wrote:
> > On 2/10/2024 12:06, Jean-Louis Dupond wrote:
> >> The filesystem list in build_fs_mount_list should skip bind mounts.
> >> This because we end up in locking situations when doing fsFreeze. Like
> >> mentioned in [1] and [2].
> >>
> >> Next to that, the build_fs_mount_list call did a fallback via
> >> build_fs_mount_list_from_mtab if mountinfo did not exist.
> >> There it skipped bind mounts, but this is broken for newer OS.
> >> This as mounts does not return the path of the bind mount but the
> >> underlying dev/partition, so S_ISDIR will never return true in
> >> dev_major_minor call.
> >>
> >> This patch simply checks the existing devmajor:devminor tuple in the
> >> mounts, and if it already exists, this means we have the same devices
> >> mounted again, a bind mount. So skip this.
> >>
> >> Same approach is used in open-vm-tools [3].
> >>
> >> [1]: https://gitlab.com/qemu-project/qemu/-/issues/592
> >> [2]: https://gitlab.com/qemu-project/qemu/-/issues/520
> >> [3]:
> >>
> https://github.com/vmware/open-vm-tools/commit/d58847b497e212737007958c945af1df22a8ab58
> >>
> >> Signed-off-by: Jean-Louis Dupond <jean-louis@dupond.be>
> >> ---
> >>   qga/commands-linux.c | 25 +++++++++++++++++++++++++
> >>   1 file changed, 25 insertions(+)
> >>
> >> diff --git a/qga/commands-linux.c b/qga/commands-linux.c
> >> index 51d5e3d927..426b040ab8 100644
> >> --- a/qga/commands-linux.c
> >> +++ b/qga/commands-linux.c
> >> @@ -59,6 +59,22 @@ static int dev_major_minor(const char *devpath,
> >>       return -1;
> >>   }
> >>   +/*
> >> + * Check if we already have the devmajor:devminor in the mounts
> >> + * If thats the case return true.
> >> + */
> >> +static bool dev_exists(FsMountList *mounts, unsigned int devmajor,
> >> unsigned int devminor)
> >> +{
> >> +    FsMount *mount;
> >> +
> >> +    QTAILQ_FOREACH(mount, mounts, next) {
> >> +        if (mount->devmajor == devmajor && mount->devminor ==
> >> devminor) {
> >> +            return true;
> >> +        }
> >> +    }
> >> +    return false;
> >> +}
> >> +
> >>   static bool build_fs_mount_list_from_mtab(FsMountList *mounts,
> >> Error **errp)
> >>   {
> >>       struct mntent *ment;
> >> @@ -89,6 +105,10 @@ static bool
> >> build_fs_mount_list_from_mtab(FsMountList *mounts, Error **errp)
> >>               /* Skip bind mounts */
> >>               continue;
> >>           }
> >> +        if (dev_exists(mounts, devmajor, devminor)) {
> >> +            /* Skip already existing devices (bind mounts) */
> >> +            continue;
> >> +        }
> >>             mount = g_new0(FsMount, 1);
> >>           mount->dirname = g_strdup(ment->mnt_dir);
> >> @@ -172,6 +192,11 @@ bool build_fs_mount_list(FsMountList *mounts,
> >> Error **errp)
> >>               }
> >>           }
> >>   +        if (dev_exists(mounts, devmajor, devminor)) {
> >> +            /* Skip already existing devices (bind mounts) */
> >> +            continue;
> >> +        }
> >> +
> >>           mount = g_new0(FsMount, 1);
> >>           mount->dirname = g_strdup(line + dir_s);
> >>           mount->devtype = g_strdup(dash + type_s);
> >
> >
> > Ping + add kkostiuk@redhat.com as I missed him in the initial mail.
> >
>
> Any chance on a review or getting it merged?
> Think it's a good (of course ;)) improvement.
>
>

[-- Attachment #2: Type: text/html, Size: 5604 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] qga: skip bind mounts in fs list
  2024-10-28 10:57     ` Konstantin Kostiuk
@ 2024-10-28 12:15       ` Jean-Louis Dupond
  2024-11-26 10:40         ` Konstantin Kostiuk
  0 siblings, 1 reply; 6+ messages in thread
From: Jean-Louis Dupond @ 2024-10-28 12:15 UTC (permalink / raw)
  To: Konstantin Kostiuk; +Cc: qemu-devel, michael.roth

[-- Attachment #1: Type: text/plain, Size: 5002 bytes --]

Hi Konstantin,

Thanks for your response.
What I observed was when running CloudLinux is that with a /tmp on a 
loop device, is that the underlying fs was first freezed, and then the 
/tmp was getting a freeze call.
But this was hanging, because it couldn't freeze as the underlying fs 
was already freezed.

So the whole system became unresponsive expect if you send a unfreeze 
via sysrq.

Build a qemu-ga with this patch included, and then it worked fine.
Because it skips the bind mounts and therefor made sure that the loop 
device was first freezed and afterwards the underlying fs.
Which works fine :)

I did not see real kernel crashes, so that was not debugged/tested.

Thanks
Jean-Louis

On 28/10/2024 11:57, Konstantin Kostiuk wrote:
> Hi Jean-Louis,
>
> Thanks for your patch. I hope next week, I will test and review this 
> patch.
>
> Just a question, did you have a chance to test that this patch fix 
> kernel crash?
>
> Best Regards,
> Konstantin Kostiuk.
>
>
> On Fri, Oct 25, 2024 at 1:06 PM Jean-Louis Dupond 
> <jean-louis@dupond.be> wrote:
>
>     On 9/10/2024 10:34, Jean-Louis Dupond wrote:
>     > On 2/10/2024 12:06, Jean-Louis Dupond wrote:
>     >> The filesystem list in build_fs_mount_list should skip bind mounts.
>     >> This because we end up in locking situations when doing
>     fsFreeze. Like
>     >> mentioned in [1] and [2].
>     >>
>     >> Next to that, the build_fs_mount_list call did a fallback via
>     >> build_fs_mount_list_from_mtab if mountinfo did not exist.
>     >> There it skipped bind mounts, but this is broken for newer OS.
>     >> This as mounts does not return the path of the bind mount but the
>     >> underlying dev/partition, so S_ISDIR will never return true in
>     >> dev_major_minor call.
>     >>
>     >> This patch simply checks the existing devmajor:devminor tuple
>     in the
>     >> mounts, and if it already exists, this means we have the same
>     devices
>     >> mounted again, a bind mount. So skip this.
>     >>
>     >> Same approach is used in open-vm-tools [3].
>     >>
>     >> [1]: https://gitlab.com/qemu-project/qemu/-/issues/592
>     >> [2]: https://gitlab.com/qemu-project/qemu/-/issues/520
>     >> [3]:
>     >>
>     https://github.com/vmware/open-vm-tools/commit/d58847b497e212737007958c945af1df22a8ab58
>     >>
>     >> Signed-off-by: Jean-Louis Dupond <jean-louis@dupond.be>
>     >> ---
>     >>   qga/commands-linux.c | 25 +++++++++++++++++++++++++
>     >>   1 file changed, 25 insertions(+)
>     >>
>     >> diff --git a/qga/commands-linux.c b/qga/commands-linux.c
>     >> index 51d5e3d927..426b040ab8 100644
>     >> --- a/qga/commands-linux.c
>     >> +++ b/qga/commands-linux.c
>     >> @@ -59,6 +59,22 @@ static int dev_major_minor(const char *devpath,
>     >>       return -1;
>     >>   }
>     >>   +/*
>     >> + * Check if we already have the devmajor:devminor in the mounts
>     >> + * If thats the case return true.
>     >> + */
>     >> +static bool dev_exists(FsMountList *mounts, unsigned int
>     devmajor,
>     >> unsigned int devminor)
>     >> +{
>     >> +    FsMount *mount;
>     >> +
>     >> +    QTAILQ_FOREACH(mount, mounts, next) {
>     >> +        if (mount->devmajor == devmajor && mount->devminor ==
>     >> devminor) {
>     >> +            return true;
>     >> +        }
>     >> +    }
>     >> +    return false;
>     >> +}
>     >> +
>     >>   static bool build_fs_mount_list_from_mtab(FsMountList *mounts,
>     >> Error **errp)
>     >>   {
>     >>       struct mntent *ment;
>     >> @@ -89,6 +105,10 @@ static bool
>     >> build_fs_mount_list_from_mtab(FsMountList *mounts, Error **errp)
>     >>               /* Skip bind mounts */
>     >>               continue;
>     >>           }
>     >> +        if (dev_exists(mounts, devmajor, devminor)) {
>     >> +            /* Skip already existing devices (bind mounts) */
>     >> +            continue;
>     >> +        }
>     >>             mount = g_new0(FsMount, 1);
>     >>           mount->dirname = g_strdup(ment->mnt_dir);
>     >> @@ -172,6 +192,11 @@ bool build_fs_mount_list(FsMountList *mounts,
>     >> Error **errp)
>     >>               }
>     >>           }
>     >>   +        if (dev_exists(mounts, devmajor, devminor)) {
>     >> +            /* Skip already existing devices (bind mounts) */
>     >> +            continue;
>     >> +        }
>     >> +
>     >>           mount = g_new0(FsMount, 1);
>     >>           mount->dirname = g_strdup(line + dir_s);
>     >>           mount->devtype = g_strdup(dash + type_s);
>     >
>     >
>     > Ping + add kkostiuk@redhat.com as I missed him in the initial mail.
>     >
>
>     Any chance on a review or getting it merged?
>     Think it's a good (of course ;)) improvement.
>

[-- Attachment #2: Type: text/html, Size: 8818 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] qga: skip bind mounts in fs list
  2024-10-28 12:15       ` Jean-Louis Dupond
@ 2024-11-26 10:40         ` Konstantin Kostiuk
  0 siblings, 0 replies; 6+ messages in thread
From: Konstantin Kostiuk @ 2024-11-26 10:40 UTC (permalink / raw)
  To: Jean-Louis Dupond; +Cc: qemu-devel, michael.roth

[-- Attachment #1: Type: text/plain, Size: 4842 bytes --]

Reviewed-by: Konstantin Kostiuk <kkostiuk@redhat.com>

I will merge it into the next QEMU release.

On Mon, Oct 28, 2024 at 2:15 PM Jean-Louis Dupond <jean-louis@dupond.be>
wrote:

> Hi Konstantin,
>
> Thanks for your response.
> What I observed was when running CloudLinux is that with a /tmp on a loop
> device, is that the underlying fs was first freezed, and then the /tmp was
> getting a freeze call.
> But this was hanging, because it couldn't freeze as the underlying fs was
> already freezed.
>
> So the whole system became unresponsive expect if you send a unfreeze via
> sysrq.
>
> Build a qemu-ga with this patch included, and then it worked fine.
> Because it skips the bind mounts and therefor made sure that the loop
> device was first freezed and afterwards the underlying fs.
> Which works fine :)
>
> I did not see real kernel crashes, so that was not debugged/tested.
>
> Thanks
> Jean-Louis
> On 28/10/2024 11:57, Konstantin Kostiuk wrote:
>
> Hi Jean-Louis,
>
> Thanks for your patch. I hope next week, I will test and review this
> patch.
>
> Just a question, did you have a chance to test that this patch fix kernel
> crash?
>
> Best Regards,
> Konstantin Kostiuk.
>
>
> On Fri, Oct 25, 2024 at 1:06 PM Jean-Louis Dupond <jean-louis@dupond.be>
> wrote:
>
>> On 9/10/2024 10:34, Jean-Louis Dupond wrote:
>> > On 2/10/2024 12:06, Jean-Louis Dupond wrote:
>> >> The filesystem list in build_fs_mount_list should skip bind mounts.
>> >> This because we end up in locking situations when doing fsFreeze. Like
>> >> mentioned in [1] and [2].
>> >>
>> >> Next to that, the build_fs_mount_list call did a fallback via
>> >> build_fs_mount_list_from_mtab if mountinfo did not exist.
>> >> There it skipped bind mounts, but this is broken for newer OS.
>> >> This as mounts does not return the path of the bind mount but the
>> >> underlying dev/partition, so S_ISDIR will never return true in
>> >> dev_major_minor call.
>> >>
>> >> This patch simply checks the existing devmajor:devminor tuple in the
>> >> mounts, and if it already exists, this means we have the same devices
>> >> mounted again, a bind mount. So skip this.
>> >>
>> >> Same approach is used in open-vm-tools [3].
>> >>
>> >> [1]: https://gitlab.com/qemu-project/qemu/-/issues/592
>> >> [2]: https://gitlab.com/qemu-project/qemu/-/issues/520
>> >> [3]:
>> >>
>> https://github.com/vmware/open-vm-tools/commit/d58847b497e212737007958c945af1df22a8ab58
>> >>
>> >> Signed-off-by: Jean-Louis Dupond <jean-louis@dupond.be>
>> >> ---
>> >>   qga/commands-linux.c | 25 +++++++++++++++++++++++++
>> >>   1 file changed, 25 insertions(+)
>> >>
>> >> diff --git a/qga/commands-linux.c b/qga/commands-linux.c
>> >> index 51d5e3d927..426b040ab8 100644
>> >> --- a/qga/commands-linux.c
>> >> +++ b/qga/commands-linux.c
>> >> @@ -59,6 +59,22 @@ static int dev_major_minor(const char *devpath,
>> >>       return -1;
>> >>   }
>> >>   +/*
>> >> + * Check if we already have the devmajor:devminor in the mounts
>> >> + * If thats the case return true.
>> >> + */
>> >> +static bool dev_exists(FsMountList *mounts, unsigned int devmajor,
>> >> unsigned int devminor)
>> >> +{
>> >> +    FsMount *mount;
>> >> +
>> >> +    QTAILQ_FOREACH(mount, mounts, next) {
>> >> +        if (mount->devmajor == devmajor && mount->devminor ==
>> >> devminor) {
>> >> +            return true;
>> >> +        }
>> >> +    }
>> >> +    return false;
>> >> +}
>> >> +
>> >>   static bool build_fs_mount_list_from_mtab(FsMountList *mounts,
>> >> Error **errp)
>> >>   {
>> >>       struct mntent *ment;
>> >> @@ -89,6 +105,10 @@ static bool
>> >> build_fs_mount_list_from_mtab(FsMountList *mounts, Error **errp)
>> >>               /* Skip bind mounts */
>> >>               continue;
>> >>           }
>> >> +        if (dev_exists(mounts, devmajor, devminor)) {
>> >> +            /* Skip already existing devices (bind mounts) */
>> >> +            continue;
>> >> +        }
>> >>             mount = g_new0(FsMount, 1);
>> >>           mount->dirname = g_strdup(ment->mnt_dir);
>> >> @@ -172,6 +192,11 @@ bool build_fs_mount_list(FsMountList *mounts,
>> >> Error **errp)
>> >>               }
>> >>           }
>> >>   +        if (dev_exists(mounts, devmajor, devminor)) {
>> >> +            /* Skip already existing devices (bind mounts) */
>> >> +            continue;
>> >> +        }
>> >> +
>> >>           mount = g_new0(FsMount, 1);
>> >>           mount->dirname = g_strdup(line + dir_s);
>> >>           mount->devtype = g_strdup(dash + type_s);
>> >
>> >
>> > Ping + add kkostiuk@redhat.com as I missed him in the initial mail.
>> >
>>
>> Any chance on a review or getting it merged?
>> Think it's a good (of course ;)) improvement.
>>
>>

[-- Attachment #2: Type: text/html, Size: 8787 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2024-11-26 10:41 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-02 10:06 [PATCH] qga: skip bind mounts in fs list Jean-Louis Dupond
2024-10-09  8:34 ` Jean-Louis Dupond
2024-10-25 10:06   ` Jean-Louis Dupond
2024-10-28 10:57     ` Konstantin Kostiuk
2024-10-28 12:15       ` Jean-Louis Dupond
2024-11-26 10:40         ` Konstantin Kostiuk

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).