| Message ID | 021e9e51-53d4-4cad-910a-56a0c431a9ef@gmail.com |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | Delta update - Issue with squashfs image source | expand |
Hi Matieu, On 18.03.24 11:28, Mathieu MEGE wrote: > Hi Stefano, > > I am currently facing an issue with squahfs images and the delta update > handler. > > The situation is: > > * Two raw partitions (p1 and p2) on my target with a squahfs image inside > * Try to update p2 with p1 as source > * p1 is already mounted > * I configured the source size as "detect" in my configuration, as I > can't be sure of it before update (very useful option) > > (Suppose to update A/B RO rootfs embedded system type) > > The delta handler tries to "mount" the "source" in order to found the > image size. Used later to compute the zck index ... > > > Here the problem comes. System refuse to mount again p1 on a tmp > directory (default rw mode) while it is already mounted in ro mode. > > In fact, everything works like a charm if we call `mount` with > `ST_READONLY` flag option. Which is a good thing in all cases (any type > of image) here. > > diff --git a/handlers/delta_handler.c b/handlers/delta_handler.c > index a5f29148..3b9a7a45 100644 > --- a/handlers/delta_handler.c > +++ b/handlers/delta_handler.c > @@ -22,6 +22,7 @@ > #include <sys/stat.h> > #include <sys/wait.h> > #include <sys/statvfs.h> > +#include <sys/mount.h> > #include <unistd.h> > #include <fcntl.h> > #include <ctype.h> > @@ -931,7 +932,7 @@static int install_delta(struct img_type *img, > if (filesystem) { > char* DATADST_DIR; > if (asprintf(&DATADST_DIR, "%s%s", get_tmpdir(), DATADST_DIR_SUFFIX) != > -1) { > - if (!swupdate_mount(priv->srcdev, DATADST_DIR, filesystem)) { > + if (!mount(priv->srcdev, DATADST_DIR, filesystem, ST_RDONLY, NULL)) { > struct statvfs vfs; > if (!statvfs(DATADST_DIR, &vfs)) { > TRACE("Detected filesystem %s, block size : %lu, %lu blocks = %lu size", > -- > > > In fact, I can see that the `mount` system call has been reimplemented > as `swupdate_mount` to be able to mount on Linux as well as on FreeBSD: > > intswupdate_mount(constchar*device, constchar*dir, constchar*fstype) > { > mount > #ifdefined(__linux__) > returnmount(device, dir, fstype, 0, NULL); > #elifdefined(__FreeBSD__) > intiovlen =8; > structiovec iov[iovlen]; > intmntflags =0; > charerrmsg[255]; > memset(errmsg, 0, sizeof(errmsg)); > iov[0].iov_base =(void*)"fstype"; > iov[0].iov_len =strlen("fstype") +1; > iov[1].iov_base =(void*)fstype; > iov[1].iov_len =strlen(fstype) +1; > iov[2].iov_base =(void*)"fspath"; > iov[2].iov_len =strlen("fspath") +1; > iov[3].iov_base =(void*)dir; > iov[3].iov_len =strlen(dir) +1; > iov[4].iov_base =(void*)"from"; > iov[4].iov_len =strlen("from") +1; > iov[5].iov_base =(void*)device; > iov[5].iov_len =strlen(device) +1; > /* The underlying fs driver may require a > buffer for an error message, even if we > do not use it here. */ > iov[6].iov_base =(void*)"errmsg"; > iov[6].iov_len =strlen("errmsg") +1; > iov[7].iov_base =errmsg; > iov[7].iov_len =strlen(errmsg) +1; > returnnmount(iov, iovlen, mntflags); > #else > /* Not implemented for this OS, no specific errno. */ > (void)device; > (void)dir; > (void)fstype; > errno =0; > return-1; > #endif > } > > This function is called a couple of times elsewhere in swupdate. > > Of course, there are different ways to treat this issue and I would like > to know what is the best according to you. > > My suggestion is:int mount(const char *type, const char *dir, int flags, void *data); > As FreeBSD supports `mount` system call the same as Linux > (https://man.freebsd.org/cgi/man.cgi?nmount(2) ), I do not see this - FreeBSD defines as : int mount(const char *type, const char *dir, int flags, void *data); Linux as: int mount(const char *source, const char *target, const char *filesystemtype, unsigned long mountflags, const void *data); Fingerprint does not match - then the reason to have a wrapper "swupdate_[u]mount". > we could remove the > "swupdate_mount" API function and replace `swupdate_(u)mount` by `mount` > from sys/mount.h and use flag options where it is necessary. > > Please let me know, your point of view, and I'll be glad to submit a > patch for this. > Best regards, Stefano > Best regards, > > Mathieu MEGE > > -- > You received this message because you are subscribed to the Google > Groups "swupdate" group. > To unsubscribe from this group and stop receiving emails from it, send > an email to swupdate+unsubscribe@googlegroups.com > <mailto:swupdate+unsubscribe@googlegroups.com>. > To view this discussion on the web visit > https://groups.google.com/d/msgid/swupdate/021e9e51-53d4-4cad-910a-56a0c431a9ef%40gmail.com <https://groups.google.com/d/msgid/swupdate/021e9e51-53d4-4cad-910a-56a0c431a9ef%40gmail.com?utm_medium=email&utm_source=footer>.
Hey Stefano, Thank you for your prompt answer. Le 18/03/2024 à 12:44, Stefano Babic a écrit : > Hi Matieu, > > On 18.03.24 11:28, Mathieu MEGE wrote: >> Hi Stefano, >> >> I am currently facing an issue with squahfs images and the delta update >> handler. >> >> The situation is: >> >> * Two raw partitions (p1 and p2) on my target with a squahfs image >> inside >> * Try to update p2 with p1 as source >> * p1 is already mounted >> * I configured the source size as "detect" in my configuration, as I >> can't be sure of it before update (very useful option) >> >> (Suppose to update A/B RO rootfs embedded system type) >> >> The delta handler tries to "mount" the "source" in order to found the >> image size. Used later to compute the zck index ... >> >> > Here the problem comes. System refuse to mount again p1 on a tmp >> directory (default rw mode) while it is already mounted in ro mode. >> >> In fact, everything works like a charm if we call `mount` with >> `ST_READONLY` flag option. Which is a good thing in all cases (any type >> of image) here. >> >> diff --git a/handlers/delta_handler.c b/handlers/delta_handler.c >> index a5f29148..3b9a7a45 100644 >> --- a/handlers/delta_handler.c >> +++ b/handlers/delta_handler.c >> @@ -22,6 +22,7 @@ >> #include <sys/stat.h> >> #include <sys/wait.h> >> #include <sys/statvfs.h> >> +#include <sys/mount.h> >> #include <unistd.h> >> #include <fcntl.h> >> #include <ctype.h> >> @@ -931,7 +932,7 @@static int install_delta(struct img_type *img, >> if (filesystem) { >> char* DATADST_DIR; >> if (asprintf(&DATADST_DIR, "%s%s", get_tmpdir(), DATADST_DIR_SUFFIX) != >> -1) { >> - if (!swupdate_mount(priv->srcdev, DATADST_DIR, filesystem)) { >> + if (!mount(priv->srcdev, DATADST_DIR, filesystem, ST_RDONLY, NULL)) { >> struct statvfs vfs; >> if (!statvfs(DATADST_DIR, &vfs)) { >> TRACE("Detected filesystem %s, block size : %lu, %lu blocks = %lu size", >> -- >> >> >> In fact, I can see that the `mount` system call has been reimplemented >> as `swupdate_mount` to be able to mount on Linux as well as on FreeBSD: >> >> intswupdate_mount(constchar*device, constchar*dir, constchar*fstype) >> { >> mount >> #ifdefined(__linux__) >> returnmount(device, dir, fstype, 0, NULL); >> #elifdefined(__FreeBSD__) >> intiovlen =8; >> structiovec iov[iovlen]; >> intmntflags =0; >> charerrmsg[255]; >> memset(errmsg, 0, sizeof(errmsg)); >> iov[0].iov_base =(void*)"fstype"; >> iov[0].iov_len =strlen("fstype") +1; >> iov[1].iov_base =(void*)fstype; >> iov[1].iov_len =strlen(fstype) +1; >> iov[2].iov_base =(void*)"fspath"; >> iov[2].iov_len =strlen("fspath") +1; >> iov[3].iov_base =(void*)dir; >> iov[3].iov_len =strlen(dir) +1; >> iov[4].iov_base =(void*)"from"; >> iov[4].iov_len =strlen("from") +1; >> iov[5].iov_base =(void*)device; >> iov[5].iov_len =strlen(device) +1; >> /* The underlying fs driver may require a >> buffer for an error message, even if we >> do not use it here. */ >> iov[6].iov_base =(void*)"errmsg"; >> iov[6].iov_len =strlen("errmsg") +1; >> iov[7].iov_base =errmsg; >> iov[7].iov_len =strlen(errmsg) +1; >> returnnmount(iov, iovlen, mntflags); >> #else >> /* Not implemented for this OS, no specific errno. */ >> (void)device; >> (void)dir; >> (void)fstype; >> errno =0; >> return-1; >> #endif >> } >> >> This function is called a couple of times elsewhere in swupdate. >> >> Of course, there are different ways to treat this issue and I would like >> to know what is the best according to you. >> >> My suggestion is:int > mount(const char *type, const char *dir, int flags, > void *data); > >> As FreeBSD supports `mount` system call the same as Linux >> (https://man.freebsd.org/cgi/man.cgi?nmount(2) ), > > I do not see this - FreeBSD defines as : > > int > mount(const char *type, const char *dir, int flags, > void *data); > > Linux as: > > int mount(const char *source, const char *target, > const char *filesystemtype, unsigned long mountflags, > const void *data); > > > Fingerprint does not match - then the reason to have a wrapper > "swupdate_[u]mount". Indeed. Did net see that the FreeBSD's `mount` is lacking of a `from` in its fingerprint. My mistake. > >> we could remove the >> "swupdate_mount" API function and replace `swupdate_(u)mount` by `mount` >> from sys/mount.h and use flag options where it is necessary. >> >> Please let me know, your point of view, and I'll be glad to submit a >> patch for this. >> > > Best regards, > Stefano > > >> Best regards, >> >> Mathieu MEGE >> >> -- >> You received this message because you are subscribed to the Google >> Groups "swupdate" group. >> To unsubscribe from this group and stop receiving emails from it, send >> an email to swupdate+unsubscribe@googlegroups.com >> <mailto:swupdate+unsubscribe@googlegroups.com>. >> To view this discussion on the web visit >> https://groups.google.com/d/msgid/swupdate/021e9e51-53d4-4cad-910a-56a0c431a9ef%40gmail.com >> <https://groups.google.com/d/msgid/swupdate/021e9e51-53d4-4cad-910a-56a0c431a9ef%40gmail.com?utm_medium=email&utm_source=footer>. >> In that case, what do you think about something like : diff --git a/core/util.c b/core/util.c index 84df7ad..65d0c22 100644 --- a/core/util.c +++ b/core/util.c @@ -753,14 +753,14 @@ bool strtobool(const char *s) return false; } -int swupdate_mount(const char *device, const char *dir, const char *fstype) +int swupdate_mount(const char *device, const char *dir, const char *fstype, const bool ro) { #if defined(__linux__) - return mount(device, dir, fstype, 0, NULL); + return mount(device, dir, fstype, ro ? ST_READONLY : 0, NULL); #elif defined(__FreeBSD__) int iovlen = 8; struct iovec iov[iovlen]; - int mntflags = 0; + int mntflags = ro ? MNT_RDONLY : 0; char errmsg[255]; memset(errmsg, 0, sizeof(errmsg)); iov[0].iov_base = (void*)"fstype"; diff --git a/handlers/archive_handler.c b/handlers/archive_handler.c index f973a27..4f4b6b3 100644 --- a/handlers/archive_handler.c +++ b/handlers/archive_handler.c @@ -257,7 +257,7 @@ static int install_archive_image(struct img_type *img, pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_JOINABLE); if (use_mount) { - ret = swupdate_mount(img->device, DATADST_DIR, img->filesystem); + ret = swupdate_mount(img->device, DATADST_DIR, img->filesystem, false); if (ret) { ERROR("Device %s with filesystem %s cannot be mounted", img->device, img->filesystem); diff --git a/handlers/btrfs_handler.c b/handlers/btrfs_handler.c index 32ecfa6..e6f0174 100644 --- a/handlers/btrfs_handler.c +++ b/handlers/btrfs_handler.c @@ -54,7 +54,7 @@ static int btrfs(struct img_type *img, sprintf(globalpath, "%s%s", get_tmpdir(), DATADST_DIR_SUFFIX); mountpoint = strdupa(globalpath); DEBUG("Try to mount %s as BTRFS", mountpoint); - ret = swupdate_mount(img->device, mountpoint, "btrfs"); + ret = swupdate_mount(img->device, mountpoint, "btrfs", false); if (ret) { ERROR("%s cannot be mounted with btrfs", img->device); return -1; diff --git a/handlers/delta_handler.c b/handlers/delta_handler.c index 5b45003..56c0e81 100644 --- a/handlers/delta_handler.c +++ b/handlers/delta_handler.c @@ -930,7 +930,7 @@ static int install_delta(struct img_type *img, if (filesystem) { char* DATADST_DIR; if (asprintf(&DATADST_DIR, "%s%s", get_tmpdir(), DATADST_DIR_SUFFIX) != -1) { - if (!swupdate_mount(priv->srcdev, DATADST_DIR, filesystem)) { + if (!swupdate_mount(priv->srcdev, DATADST_DIR, filesystem, true)) { struct statvfs vfs; if (!statvfs(DATADST_DIR, &vfs)) { TRACE("Detected filesystem %s, block size : %lu, %lu blocks = %lu size", diff --git a/handlers/raw_handler.c b/handlers/raw_handler.c Once again, if you have a better way in mind to solve this issue, feel free to tell me about it. Best regards, Mathieu,
diff --git a/handlers/delta_handler.c b/handlers/delta_handler.c index a5f29148..3b9a7a45 100644 --- a/handlers/delta_handler.c +++ b/handlers/delta_handler.c @@ -22,6 +22,7 @@ #include <sys/stat.h> #include <sys/wait.h> #include <sys/statvfs.h> +#include <sys/mount.h> #include <unistd.h> #include <fcntl.h> #include <ctype.h> @@ -931,7 +932,7 @@ static int install_delta(struct img_type *img, if (filesystem) { char* DATADST_DIR; if (asprintf(&DATADST_DIR, "%s%s", get_tmpdir(), DATADST_DIR_SUFFIX) != -1) { - if (!swupdate_mount(priv->srcdev, DATADST_DIR, filesystem)) { + if (!mount(priv->srcdev, DATADST_DIR, filesystem, ST_RDONLY, NULL)) { struct statvfs vfs; if (!statvfs(DATADST_DIR, &vfs)) { TRACE("Detected filesystem %s, block size : %lu, %lu blocks = %lu size",
Hi Stefano, I am currently facing an issue with squahfs images and the delta update handler. The situation is: * Two raw partitions (p1 and p2) on my target with a squahfs image inside * Try to update p2 with p1 as source * p1 is already mounted * I configured the source size as "detect" in my configuration, as I can't be sure of it before update (very useful option) (Suppose to update A/B RO rootfs embedded system type) The delta handler tries to "mount" the "source" in order to found the image size. Used later to compute the zck index ... > Here the problem comes. System refuse to mount again p1 on a tmp directory (default rw mode) while it is already mounted in ro mode. In fact, everything works like a charm if we call `mount` with `ST_READONLY` flag option. Which is a good thing in all cases (any type of image) here.