<div dir="ltr"><div dir="ltr"><div class="gmail_default" style="font-size:small">Cyril Hrubis <<a href="mailto:chrubis@suse.cz" target="_blank">chrubis@suse.cz</a>> wrote:<br></div></div><div class="gmail_quote"><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> +char *limit_tmpfs_mount_size(const char *mnt_data, unsigned int size);<br>
<br>
If we want this function to be public it has to be prefixed with 'tst_'.<br>
<br>
Also do we really need this to be public?<br></blockquote><div><br></div><div><div class="gmail_default" style="font-size:small">No, we don't. I put it in the tst_device.c only hope to make use of the </div><div class="gmail_default" style="font-size:small">DEV_SIZE_MB definition, and now seems this is a bad idea.<br></div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small">Looks like we should move it back to tst_test.c and state it as a static function.</div></div><div> </div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> <br>
> +static char tmpfs_buf[1024];<br>
<br>
Can we please, instead of adding a global variable, pass the buffer and<br>
it's size to the limit_tmpfs_mount size, and then create the path on the<br>
stack in the prepare device function?<br>
<br>
</blockquote><div><br></div><div><div class="gmail_default" style="font-size:small">Sure.</div></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> <br>
> +char *limit_tmpfs_mount_size(const char *mnt_data, unsigned int size)<br>
> +{<br>
> +     unsigned int dev_size = MAX(size, DEV_SIZE_MB);<br>
> +<br>
> +     if (mnt_data)<br>
> +             snprintf(tmpfs_buf, sizeof(tmpfs_buf), "%s,size=%uM", mnt_data, dev_size);<br>
> +     else<br>
> +             snprintf(tmpfs_buf, sizeof(tmpfs_buf), "size=%uM", dev_size);<br>
> +<br>
> +     tst_resm(TINFO, "Limiting tmpfs size to %uMB", dev_size);<br>
> +<br>
> +     return tmpfs_buf;<br>
> +}<br>
<br>
If we passed the filesystem type to this function here as well we could<br>
do:<br>
<br>
        if (!strcmp(fs_type, "tmpfs"))<br>
                return mnt_data;<br>
<br>
As a first thing in this function and then we could pass the return<br>
value from this function to the SAFE_MOUNT() unconditionally.<br></blockquote><div><br></div><div><div class="gmail_default" style="font-size:small">+1 This sounds good.</div></div><div>  </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> +<br>
> +             if (!strcmp(tdev.fs_type, "tmpfs"))<br>
> +                     tst_test->mnt_data = mnt_data;<br>
<br>
I guess that we are doing this in order to export the changes in the<br>
mnt_data to the test, right?<br>
<br>
Is that needed for something or are you doing this just in a case that<br>
somebody will use that?<br></blockquote><div><br></div><div class="gmail_default" style="font-size:small">No, you probably mis-read this part. </div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small">In contrast, this is just to restore it to the original value, </div><div class="gmail_default" style="font-size:small">because we don't want to export the changed tst_test->mnt_data</div><div class="gmail_default" style="font-size:small">take effect on other filesystems.</div></div><div><br></div>-- <br><div dir="ltr"><div dir="ltr"><div>Regards,<br></div><div>Li Wang<br></div></div></div></div>