[libvirt] [PATCH 0/2] qemu: driver: Document and fix usage of qemuOpenFile
Peter Krempa (2): qemu: driver: Document qemuOpenFile qemu: driver: Fix usage of qemuOpenFile src/qemu/qemu_driver.c | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) -- 2.12.2
The function is nontrivial to follow and has non-standard return values. Recent usage was buggy. --- src/qemu/qemu_driver.c | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index a5c664e65..92ef983ae 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2862,13 +2862,32 @@ qemuCompressGetCommand(virQEMUSaveFormat compression) return ret; } -/* Internal function to properly create or open existing files, with - * ownership affected by qemu driver setup and domain DAC label. */ +/** + * qemuOpenFile: + * @driver: driver object + * @vm: domain object + * @path: path to file to open + * @oflags: flags for opening/creation of the file + * @needUnlink: set to true if file was created by this function + * @bypassSecurityDriver: optional pointer to a boolean that will be set to true + * if security driver operations are pointless (due to + * NFS mount) + * + * Internal function to properly create or open existing files, with + * ownership affected by qemu driver setup and domain DAC label. + * + * Returns the file descriptor on success and negative errno on failure. + * + * This function should not be used on storage sources. Use + * qemuDomainStorageFileInit and storage driver APIs if possible. + **/ static int qemuOpenFile(virQEMUDriverPtr driver, virDomainObjPtr vm, - const char *path, int oflags, - bool *needUnlink, bool *bypassSecurityDriver) + const char *path, + int oflags, + bool *needUnlink, + bool *bypassSecurityDriver) { int ret = -1; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); -- 2.12.2
The function returns -errno on failure, not only -1. This broken usage was introduced by commit 732af77cce. --- src/qemu/qemu_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 92ef983ae..b238181f3 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11327,7 +11327,7 @@ qemuDomainStorageOpenStat(virQEMUDriverPtr driver, { if (virStorageSourceIsLocalStorage(src)) { if ((*ret_fd = qemuOpenFile(driver, vm, src->path, O_RDONLY, - NULL, NULL)) == -1) + NULL, NULL)) < 0) return -1; if (fstat(*ret_fd, ret_sb) < 0) { -- 2.12.2
On Wed, May 10, 2017 at 02:18:33PM +0200, Peter Krempa wrote:
The function returns -errno on failure, not only -1. This broken usage was introduced by commit 732af77cce.
Actually, that was commit b4a40dd92dc7e6f110b13f2353cb5343d1147227 I am unsure why this function should return -errno since no caller cares.
--- src/qemu/qemu_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
ACK series if you fix the usage in qemuDomainBlockPeek as well. Jan
participants (2)
-
Ján Tomko -
Peter Krempa