More posix_fadvise stuff, and document what Linux really does with these calls. Also fixes a nasty bug in virt-builder. Rich.
Richard W.M. Jones
2016-Apr-14 15:20 UTC
[Libguestfs] [PATCH 1/3] fadvise: Add guestfs_int_fadvise_normal, document Linux behaviour.
This commit adds guestfs_int_fadvise_normal, but it's not enabled since nothing calls it. It also documents what Linux actually does, which is a bit different from what I thought these settings did. Note this is for Linux 4.6.0rc3 and may change in future. This updates commit 83e92b4a972a4dafdcc42388f4a5394e804dd7bf. --- src/guestfs-internal-frontend.h | 1 + src/utils.c | 57 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+) diff --git a/src/guestfs-internal-frontend.h b/src/guestfs-internal-frontend.h index 5f16647..b52b584 100644 --- a/src/guestfs-internal-frontend.h +++ b/src/guestfs-internal-frontend.h @@ -87,6 +87,7 @@ extern int guestfs_int_is_true (const char *str); extern const char *guestfs_int_ovmf_i386_firmware[]; extern const char *guestfs_int_ovmf_x86_64_firmware[]; extern const char *guestfs_int_aavmf_firmware[]; +//extern void guestfs_int_fadvise_normal (int fd); extern void guestfs_int_fadvise_sequential (int fd); extern void guestfs_int_fadvise_random (int fd); extern void guestfs_int_fadvise_noreuse (int fd); diff --git a/src/utils.c b/src/utils.c index 241e745..79adf20 100644 --- a/src/utils.c +++ b/src/utils.c @@ -388,9 +388,36 @@ guestfs_int_aavmf_firmware[] = { NULL }; +#if 0 /* not used yet */ +/** + * Hint that we will read or write the file descriptor normally. + * + * On Linux, this clears the C<FMODE_RANDOM> flag on the file [see + * below] and sets the per-file number of readahead pages to equal the + * block device readahead setting. + * + * It's OK to call this on a non-file since we ignore failure as it is + * only a hint. + */ +void +guestfs_int_fadvise_normal (int fd) +{ +#if defined(HAVE_POSIX_FADVISE) && defined(POSIX_FADV_NORMAL) + /* It's not clear from the man page, but the 'advice' parameter is + * NOT a bitmask. You can only pass one parameter with each call. + */ + ignore_value (posix_fadvise (fd, 0, 0, POSIX_FADV_NORMAL)); +#endif +} +#endif + /** * Hint that we will read or write the file descriptor sequentially. * + * On Linux, this clears the C<FMODE_RANDOM> flag on the file [see + * below] and sets the per-file number of readahead pages to twice the + * block device readahead setting. + * * It's OK to call this on a non-file since we ignore failure as it is * only a hint. */ @@ -408,6 +435,23 @@ guestfs_int_fadvise_sequential (int fd) /** * Hint that we will read or write the file descriptor randomly. * + * On Linux, this sets the C<FMODE_RANDOM> flag on the file. The + * effect of this flag is to: + * + * =over 4 + * + * =item * + * + * Disable normal sequential file readahead. + * + * =item * + * + * If any read of the file is done which misses in the page cache, 2MB + * are read into the page cache. [I think - I'm not sure I totally + * understand what this is doing] + * + * =back + * * It's OK to call this on a non-file since we ignore failure as it is * only a hint. */ @@ -425,6 +469,8 @@ guestfs_int_fadvise_random (int fd) /** * Hint that we will access the data only once. * + * On Linux, this does nothing. + * * It's OK to call this on a non-file since we ignore failure as it is * only a hint. */ @@ -443,6 +489,13 @@ guestfs_int_fadvise_noreuse (int fd) /** * Hint that we will not access the data in the near future. * + * On Linux, this immediately writes out any dirty pages in the page + * cache and then invalidates (drops) all pages associated with this + * file from the page cache. Apparently it does this even if the file + * is opened or being used by other processes. This setting is not + * persistent; if you subsequently read the file it will be cached in + * the page cache as normal. + * * It's OK to call this on a non-file since we ignore failure as it is * only a hint. */ @@ -461,6 +514,10 @@ guestfs_int_fadvise_dontneed (int fd) /** * Hint that we will access the data in the near future. * + * On Linux, this immediately reads the whole file into the page + * cache. This setting is not persistent; subsequently pages may be + * dropped from the page cache as normal. + * * It's OK to call this on a non-file since we ignore failure as it is * only a hint. */ -- 2.7.4
Richard W.M. Jones
2016-Apr-14 15:20 UTC
[Libguestfs] [PATCH 2/3] builder: pxzcat: Close the output file.
After uncompressing the template we didn't close the output file, which potentially could cause writes to the output file to be lost. --- builder/pxzcat-c.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/builder/pxzcat-c.c b/builder/pxzcat-c.c index ae9a060..ce0516c 100644 --- a/builder/pxzcat-c.c +++ b/builder/pxzcat-c.c @@ -227,6 +227,9 @@ pxzcat (value filenamev, value outputfilev, unsigned nr_threads) if (close (fd) == -1) unix_error (errno, (char *) "close", filenamev); + + if (close (ofd) == -1) + unix_error (errno, (char *) "close", outputfilev); } static int -- 2.7.4
Richard W.M. Jones
2016-Apr-14 15:20 UTC
[Libguestfs] [PATCH 3/3] builder: pxzcat: Remove ineffective POSIX_FADV_WILLNEED.
On Linux this will load the whole file into the page cache. However the output file is empty and zero sized just after it is opened, so this has no effect. Note that the advice is not persistent, so this really does nothing. I considered adding the call back after the file has been written, just before the close, but: - If we do a virt-resize next then we will open and read the file mostly sequentially, so readahead will deal with any missing pages. - If we do a virt-customize next then we will only access a small part of the disk image, so loading it all into the page cache adds extra work. - In any case, since we have just written the file it's likely to still be in the page cache. --- builder/pxzcat-c.c | 1 - 1 file changed, 1 deletion(-) diff --git a/builder/pxzcat-c.c b/builder/pxzcat-c.c index ce0516c..fa9f6bf 100644 --- a/builder/pxzcat-c.c +++ b/builder/pxzcat-c.c @@ -194,7 +194,6 @@ pxzcat (value filenamev, value outputfilev, unsigned nr_threads) } guestfs_int_fadvise_random (ofd); - guestfs_int_fadvise_willneed (ofd); if (ftruncate (ofd, 1) == -1) { int err = errno; -- 2.7.4