* [Qemu-devel] [PATCH v2 0/4] QIOChannelFile bug fixes
@ 2017-11-01 14:25 Ross Lagerwall
2017-11-01 14:25 ` [Qemu-devel] [PATCH v2 1/4] migration: Don't leak IO channels Ross Lagerwall
` (5 more replies)
0 siblings, 6 replies; 16+ messages in thread
From: Ross Lagerwall @ 2017-11-01 14:25 UTC (permalink / raw)
To: qemu-devel; +Cc: Ross Lagerwall, Daniel P. Berrange
Hi,
Here is a bug fix with the use of QIOChannelFile and 2 bug fixes and an
improvement to implementation of QIOChannelFile.
Regards,
Ross Lagerwall
Ross Lagerwall (4):
migration: Don't leak IO channels
io: Fix QIOChannelFile when creating and opening read-write
io: Don't call close multiple times in QIOChannelFile
io: Add /dev/fdset/ support to QIOChannelFile
include/io/channel-file.h | 2 +-
io/channel-file.c | 11 ++++-------
migration/savevm.c | 2 ++
tests/test-io-channel-file.c | 29 +++++++++++++++++++++++++----
4 files changed, 32 insertions(+), 12 deletions(-)
--
2.9.5
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH v2 1/4] migration: Don't leak IO channels
2017-11-01 14:25 [Qemu-devel] [PATCH v2 0/4] QIOChannelFile bug fixes Ross Lagerwall
@ 2017-11-01 14:25 ` Ross Lagerwall
2018-01-18 13:41 ` Ross Lagerwall
` (2 more replies)
2017-11-01 14:25 ` [Qemu-devel] [PATCH v2 2/4] io: Fix QIOChannelFile when creating and opening read-write Ross Lagerwall
` (4 subsequent siblings)
5 siblings, 3 replies; 16+ messages in thread
From: Ross Lagerwall @ 2017-11-01 14:25 UTC (permalink / raw)
To: qemu-devel; +Cc: Ross Lagerwall, Juan Quintela, Dr. David Alan Gilbert
Since qemu_fopen_channel_{in,out}put take references on the underlying
IO channels, make sure to release our references to them.
Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
---
New in v2.
migration/savevm.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/migration/savevm.c b/migration/savevm.c
index 4a88228..87557dd 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2259,6 +2259,7 @@ void qmp_xen_save_devices_state(const char *filename, Error **errp)
}
qio_channel_set_name(QIO_CHANNEL(ioc), "migration-xen-save-state");
f = qemu_fopen_channel_output(QIO_CHANNEL(ioc));
+ object_unref(OBJECT(ioc));
ret = qemu_save_device_state(f);
qemu_fclose(f);
if (ret < 0) {
@@ -2292,6 +2293,7 @@ void qmp_xen_load_devices_state(const char *filename, Error **errp)
}
qio_channel_set_name(QIO_CHANNEL(ioc), "migration-xen-load-state");
f = qemu_fopen_channel_input(QIO_CHANNEL(ioc));
+ object_unref(OBJECT(ioc));
ret = qemu_loadvm_state(f);
qemu_fclose(f);
--
2.9.5
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH v2 2/4] io: Fix QIOChannelFile when creating and opening read-write
2017-11-01 14:25 [Qemu-devel] [PATCH v2 0/4] QIOChannelFile bug fixes Ross Lagerwall
2017-11-01 14:25 ` [Qemu-devel] [PATCH v2 1/4] migration: Don't leak IO channels Ross Lagerwall
@ 2017-11-01 14:25 ` Ross Lagerwall
2018-01-18 13:49 ` Daniel P. Berrange
2017-11-01 14:25 ` [Qemu-devel] [PATCH v2 3/4] io: Don't call close multiple times in QIOChannelFile Ross Lagerwall
` (3 subsequent siblings)
5 siblings, 1 reply; 16+ messages in thread
From: Ross Lagerwall @ 2017-11-01 14:25 UTC (permalink / raw)
To: qemu-devel; +Cc: Ross Lagerwall, Daniel P. Berrange
The code wrongly passes the mode to open() only if O_WRONLY is set.
Instead, the mode should be passed when O_CREAT is set (or O_TMPFILE on
Linux). Fix this by always passing the mode since open() will correctly
ignore the mode if it is not needed. Add a testcase which exercises this
bug and also change the existing testcase to check that the mode of the
created file is correct.
Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
---
Changed in v2:
* Separated from qemu_open() change.
include/io/channel-file.h | 2 +-
io/channel-file.c | 6 +-----
tests/test-io-channel-file.c | 29 +++++++++++++++++++++++++----
3 files changed, 27 insertions(+), 10 deletions(-)
diff --git a/include/io/channel-file.h b/include/io/channel-file.h
index 79245f1..ebfe54e 100644
--- a/include/io/channel-file.h
+++ b/include/io/channel-file.h
@@ -73,7 +73,7 @@ qio_channel_file_new_fd(int fd);
* qio_channel_file_new_path:
* @path: the file path
* @flags: the open flags (O_RDONLY|O_WRONLY|O_RDWR, etc)
- * @mode: the file creation mode if O_WRONLY is set in @flags
+ * @mode: the file creation mode if O_CREAT is set in @flags
* @errp: pointer to initialized error object
*
* Create a new IO channel object for a file represented
diff --git a/io/channel-file.c b/io/channel-file.c
index b383273..16bf7ed 100644
--- a/io/channel-file.c
+++ b/io/channel-file.c
@@ -50,11 +50,7 @@ qio_channel_file_new_path(const char *path,
ioc = QIO_CHANNEL_FILE(object_new(TYPE_QIO_CHANNEL_FILE));
- if (flags & O_WRONLY) {
- ioc->fd = open(path, flags, mode);
- } else {
- ioc->fd = open(path, flags);
- }
+ ioc->fd = open(path, flags, mode);
if (ioc->fd < 0) {
object_unref(OBJECT(ioc));
error_setg_errno(errp, errno,
diff --git a/tests/test-io-channel-file.c b/tests/test-io-channel-file.c
index 6bfede6..2e94f63 100644
--- a/tests/test-io-channel-file.c
+++ b/tests/test-io-channel-file.c
@@ -24,16 +24,21 @@
#include "io-channel-helpers.h"
#include "qapi/error.h"
-static void test_io_channel_file(void)
+#define TEST_FILE "tests/test-io-channel-file.txt"
+#define TEST_MASK 0600
+
+static void test_io_channel_file_helper(int flags)
{
QIOChannel *src, *dst;
QIOChannelTest *test;
+ struct stat st;
+ mode_t mask;
+ int ret;
-#define TEST_FILE "tests/test-io-channel-file.txt"
unlink(TEST_FILE);
src = QIO_CHANNEL(qio_channel_file_new_path(
TEST_FILE,
- O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, 0600,
+ flags, TEST_MASK,
&error_abort));
dst = QIO_CHANNEL(qio_channel_file_new_path(
TEST_FILE,
@@ -45,18 +50,33 @@ static void test_io_channel_file(void)
qio_channel_test_run_reader(test, dst);
qio_channel_test_validate(test);
+ /* Check that the requested mode took effect. */
+ mask = umask(0);
+ umask(mask);
+ ret = stat(TEST_FILE, &st);
+ g_assert_cmpint(ret, >, -1);
+ g_assert_cmpuint(TEST_MASK & ~mask, ==, st.st_mode & 0777);
+
unlink(TEST_FILE);
object_unref(OBJECT(src));
object_unref(OBJECT(dst));
}
+static void test_io_channel_file(void)
+{
+ test_io_channel_file_helper(O_WRONLY | O_CREAT | O_TRUNC | O_BINARY);
+}
+
+static void test_io_channel_file_rdwr(void)
+{
+ test_io_channel_file_helper(O_RDWR | O_CREAT | O_TRUNC | O_BINARY);
+}
static void test_io_channel_fd(void)
{
QIOChannel *ioc;
int fd = -1;
-#define TEST_FILE "tests/test-io-channel-file.txt"
fd = open(TEST_FILE, O_CREAT | O_TRUNC | O_WRONLY, 0600);
g_assert_cmpint(fd, >, -1);
@@ -114,6 +134,7 @@ int main(int argc, char **argv)
g_test_init(&argc, &argv, NULL);
g_test_add_func("/io/channel/file", test_io_channel_file);
+ g_test_add_func("/io/channel/file/rdwr", test_io_channel_file_rdwr);
g_test_add_func("/io/channel/file/fd", test_io_channel_fd);
#ifndef _WIN32
g_test_add_func("/io/channel/pipe/sync", test_io_channel_pipe_sync);
--
2.9.5
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH v2 3/4] io: Don't call close multiple times in QIOChannelFile
2017-11-01 14:25 [Qemu-devel] [PATCH v2 0/4] QIOChannelFile bug fixes Ross Lagerwall
2017-11-01 14:25 ` [Qemu-devel] [PATCH v2 1/4] migration: Don't leak IO channels Ross Lagerwall
2017-11-01 14:25 ` [Qemu-devel] [PATCH v2 2/4] io: Fix QIOChannelFile when creating and opening read-write Ross Lagerwall
@ 2017-11-01 14:25 ` Ross Lagerwall
2017-11-01 18:11 ` Marc-André Lureau
2018-01-18 13:50 ` Daniel P. Berrange
2017-11-01 14:25 ` [Qemu-devel] [PATCH v2 4/4] io: Add /dev/fdset/ support to QIOChannelFile Ross Lagerwall
` (2 subsequent siblings)
5 siblings, 2 replies; 16+ messages in thread
From: Ross Lagerwall @ 2017-11-01 14:25 UTC (permalink / raw)
To: qemu-devel; +Cc: Ross Lagerwall, Daniel P. Berrange
If the file descriptor underlying QIOChannelFile is closed in the
io_close() method, don't close it again in the finalize() method since
the file descriptor number may have been reused in the meantime.
Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
---
New in v2.
io/channel-file.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/io/channel-file.c b/io/channel-file.c
index 16bf7ed..1f2f710 100644
--- a/io/channel-file.c
+++ b/io/channel-file.c
@@ -178,6 +178,7 @@ static int qio_channel_file_close(QIOChannel *ioc,
"Unable to close file");
return -1;
}
+ fioc->fd = -1;
return 0;
}
--
2.9.5
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH v2 4/4] io: Add /dev/fdset/ support to QIOChannelFile
2017-11-01 14:25 [Qemu-devel] [PATCH v2 0/4] QIOChannelFile bug fixes Ross Lagerwall
` (2 preceding siblings ...)
2017-11-01 14:25 ` [Qemu-devel] [PATCH v2 3/4] io: Don't call close multiple times in QIOChannelFile Ross Lagerwall
@ 2017-11-01 14:25 ` Ross Lagerwall
2017-11-01 18:12 ` Marc-André Lureau
2018-01-18 13:50 ` Daniel P. Berrange
2018-01-18 13:40 ` [Qemu-devel] [PATCH v2 0/4] QIOChannelFile bug fixes Ross Lagerwall
2018-01-18 13:51 ` Daniel P. Berrange
5 siblings, 2 replies; 16+ messages in thread
From: Ross Lagerwall @ 2017-11-01 14:25 UTC (permalink / raw)
To: qemu-devel; +Cc: Ross Lagerwall, Daniel P. Berrange
Add /dev/fdset/ support to QIOChannelFile by calling qemu_open() instead
of open() and qemu_close() instead of close(). There is a subtle
semantic change since qemu_open() automatically sets O_CLOEXEC, but this
doesn't affect any of the users of the function.
Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
---
Changed in v2:
* Split into separate patch.
io/channel-file.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/io/channel-file.c b/io/channel-file.c
index 1f2f710..db948ab 100644
--- a/io/channel-file.c
+++ b/io/channel-file.c
@@ -50,7 +50,7 @@ qio_channel_file_new_path(const char *path,
ioc = QIO_CHANNEL_FILE(object_new(TYPE_QIO_CHANNEL_FILE));
- ioc->fd = open(path, flags, mode);
+ ioc->fd = qemu_open(path, flags, mode);
if (ioc->fd < 0) {
object_unref(OBJECT(ioc));
error_setg_errno(errp, errno,
@@ -74,7 +74,7 @@ static void qio_channel_file_finalize(Object *obj)
{
QIOChannelFile *ioc = QIO_CHANNEL_FILE(obj);
if (ioc->fd != -1) {
- close(ioc->fd);
+ qemu_close(ioc->fd);
ioc->fd = -1;
}
}
@@ -173,7 +173,7 @@ static int qio_channel_file_close(QIOChannel *ioc,
{
QIOChannelFile *fioc = QIO_CHANNEL_FILE(ioc);
- if (close(fioc->fd) < 0) {
+ if (qemu_close(fioc->fd) < 0) {
error_setg_errno(errp, errno,
"Unable to close file");
return -1;
--
2.9.5
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/4] io: Don't call close multiple times in QIOChannelFile
2017-11-01 14:25 ` [Qemu-devel] [PATCH v2 3/4] io: Don't call close multiple times in QIOChannelFile Ross Lagerwall
@ 2017-11-01 18:11 ` Marc-André Lureau
2018-01-18 13:50 ` Daniel P. Berrange
1 sibling, 0 replies; 16+ messages in thread
From: Marc-André Lureau @ 2017-11-01 18:11 UTC (permalink / raw)
To: Ross Lagerwall; +Cc: QEMU
On Wed, Nov 1, 2017 at 3:25 PM, Ross Lagerwall
<ross.lagerwall@citrix.com> wrote:
> If the file descriptor underlying QIOChannelFile is closed in the
> io_close() method, don't close it again in the finalize() method since
> the file descriptor number may have been reused in the meantime.
>
> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> New in v2.
>
> io/channel-file.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/io/channel-file.c b/io/channel-file.c
> index 16bf7ed..1f2f710 100644
> --- a/io/channel-file.c
> +++ b/io/channel-file.c
> @@ -178,6 +178,7 @@ static int qio_channel_file_close(QIOChannel *ioc,
> "Unable to close file");
> return -1;
> }
> + fioc->fd = -1;
> return 0;
> }
>
> --
> 2.9.5
>
>
--
Marc-André Lureau
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH v2 4/4] io: Add /dev/fdset/ support to QIOChannelFile
2017-11-01 14:25 ` [Qemu-devel] [PATCH v2 4/4] io: Add /dev/fdset/ support to QIOChannelFile Ross Lagerwall
@ 2017-11-01 18:12 ` Marc-André Lureau
2018-01-18 13:50 ` Daniel P. Berrange
1 sibling, 0 replies; 16+ messages in thread
From: Marc-André Lureau @ 2017-11-01 18:12 UTC (permalink / raw)
To: Ross Lagerwall; +Cc: QEMU
On Wed, Nov 1, 2017 at 3:25 PM, Ross Lagerwall
<ross.lagerwall@citrix.com> wrote:
> Add /dev/fdset/ support to QIOChannelFile by calling qemu_open() instead
> of open() and qemu_close() instead of close(). There is a subtle
> semantic change since qemu_open() automatically sets O_CLOEXEC, but this
> doesn't affect any of the users of the function.
>
> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> Changed in v2:
> * Split into separate patch.
>
> io/channel-file.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/io/channel-file.c b/io/channel-file.c
> index 1f2f710..db948ab 100644
> --- a/io/channel-file.c
> +++ b/io/channel-file.c
> @@ -50,7 +50,7 @@ qio_channel_file_new_path(const char *path,
>
> ioc = QIO_CHANNEL_FILE(object_new(TYPE_QIO_CHANNEL_FILE));
>
> - ioc->fd = open(path, flags, mode);
> + ioc->fd = qemu_open(path, flags, mode);
> if (ioc->fd < 0) {
> object_unref(OBJECT(ioc));
> error_setg_errno(errp, errno,
> @@ -74,7 +74,7 @@ static void qio_channel_file_finalize(Object *obj)
> {
> QIOChannelFile *ioc = QIO_CHANNEL_FILE(obj);
> if (ioc->fd != -1) {
> - close(ioc->fd);
> + qemu_close(ioc->fd);
> ioc->fd = -1;
> }
> }
> @@ -173,7 +173,7 @@ static int qio_channel_file_close(QIOChannel *ioc,
> {
> QIOChannelFile *fioc = QIO_CHANNEL_FILE(ioc);
>
> - if (close(fioc->fd) < 0) {
> + if (qemu_close(fioc->fd) < 0) {
> error_setg_errno(errp, errno,
> "Unable to close file");
> return -1;
> --
> 2.9.5
>
>
--
Marc-André Lureau
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/4] QIOChannelFile bug fixes
2017-11-01 14:25 [Qemu-devel] [PATCH v2 0/4] QIOChannelFile bug fixes Ross Lagerwall
` (3 preceding siblings ...)
2017-11-01 14:25 ` [Qemu-devel] [PATCH v2 4/4] io: Add /dev/fdset/ support to QIOChannelFile Ross Lagerwall
@ 2018-01-18 13:40 ` Ross Lagerwall
2018-01-18 13:47 ` Daniel P. Berrange
2018-01-18 13:51 ` Daniel P. Berrange
5 siblings, 1 reply; 16+ messages in thread
From: Ross Lagerwall @ 2018-01-18 13:40 UTC (permalink / raw)
To: qemu-devel
On 11/01/2017 02:25 PM, Ross Lagerwall wrote:
> Hi,
>
> Here is a bug fix with the use of QIOChannelFile and 2 bug fixes and an
> improvement to implementation of QIOChannelFile.
>
> Regards,
> Ross Lagerwall
>
> Ross Lagerwall (4):
> migration: Don't leak IO channels
> io: Fix QIOChannelFile when creating and opening read-write
> io: Don't call close multiple times in QIOChannelFile
> io: Add /dev/fdset/ support to QIOChannelFile
>
> include/io/channel-file.h | 2 +-
> io/channel-file.c | 11 ++++-------
> migration/savevm.c | 2 ++
> tests/test-io-channel-file.c | 29 +++++++++++++++++++++++++----
> 4 files changed, 32 insertions(+), 12 deletions(-)
>
Ping for reviews...
v1:
Got feedback from Daniel P. Berrange and Marc-André Lureau.
v2:
Patch 1: Unreviewed
Patch 2: Unreviewed
Patch 3: Reviewed by Marc-André Lureau
Patch 4: Reviewed by Marc-André Lureau
The patch series still applies cleanly on top of master.
Thanks,
--
Ross Lagerwall
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/4] migration: Don't leak IO channels
2017-11-01 14:25 ` [Qemu-devel] [PATCH v2 1/4] migration: Don't leak IO channels Ross Lagerwall
@ 2018-01-18 13:41 ` Ross Lagerwall
2018-01-18 13:44 ` Daniel P. Berrange
2018-02-06 11:17 ` Dr. David Alan Gilbert
2 siblings, 0 replies; 16+ messages in thread
From: Ross Lagerwall @ 2018-01-18 13:41 UTC (permalink / raw)
To: qemu-devel; +Cc: Juan Quintela, Dr. David Alan Gilbert
On 11/01/2017 02:25 PM, Ross Lagerwall wrote:
> Since qemu_fopen_channel_{in,out}put take references on the underlying
> IO channels, make sure to release our references to them.
>
> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
> ---
> New in v2.
>
> migration/savevm.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 4a88228..87557dd 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -2259,6 +2259,7 @@ void qmp_xen_save_devices_state(const char *filename, Error **errp)
> }
> qio_channel_set_name(QIO_CHANNEL(ioc), "migration-xen-save-state");
> f = qemu_fopen_channel_output(QIO_CHANNEL(ioc));
> + object_unref(OBJECT(ioc));
> ret = qemu_save_device_state(f);
> qemu_fclose(f);
> if (ret < 0) {
> @@ -2292,6 +2293,7 @@ void qmp_xen_load_devices_state(const char *filename, Error **errp)
> }
> qio_channel_set_name(QIO_CHANNEL(ioc), "migration-xen-load-state");
> f = qemu_fopen_channel_input(QIO_CHANNEL(ioc));
> + object_unref(OBJECT(ioc));
>
> ret = qemu_loadvm_state(f);
> qemu_fclose(f);
>
Ping for review...
Thanks,
--
Ross Lagerwall
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/4] migration: Don't leak IO channels
2017-11-01 14:25 ` [Qemu-devel] [PATCH v2 1/4] migration: Don't leak IO channels Ross Lagerwall
2018-01-18 13:41 ` Ross Lagerwall
@ 2018-01-18 13:44 ` Daniel P. Berrange
2018-02-06 11:17 ` Dr. David Alan Gilbert
2 siblings, 0 replies; 16+ messages in thread
From: Daniel P. Berrange @ 2018-01-18 13:44 UTC (permalink / raw)
To: Ross Lagerwall; +Cc: qemu-devel, Dr. David Alan Gilbert, Juan Quintela
On Wed, Nov 01, 2017 at 02:25:23PM +0000, Ross Lagerwall wrote:
> Since qemu_fopen_channel_{in,out}put take references on the underlying
> IO channels, make sure to release our references to them.
>
> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
> ---
> New in v2.
>
> migration/savevm.c | 2 ++
> 1 file changed, 2 insertions(+)
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
>
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 4a88228..87557dd 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -2259,6 +2259,7 @@ void qmp_xen_save_devices_state(const char *filename, Error **errp)
> }
> qio_channel_set_name(QIO_CHANNEL(ioc), "migration-xen-save-state");
> f = qemu_fopen_channel_output(QIO_CHANNEL(ioc));
> + object_unref(OBJECT(ioc));
> ret = qemu_save_device_state(f);
> qemu_fclose(f);
> if (ret < 0) {
> @@ -2292,6 +2293,7 @@ void qmp_xen_load_devices_state(const char *filename, Error **errp)
> }
> qio_channel_set_name(QIO_CHANNEL(ioc), "migration-xen-load-state");
> f = qemu_fopen_channel_input(QIO_CHANNEL(ioc));
> + object_unref(OBJECT(ioc));
>
> ret = qemu_loadvm_state(f);
> qemu_fclose(f);
> --
> 2.9.5
>
>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/4] QIOChannelFile bug fixes
2018-01-18 13:40 ` [Qemu-devel] [PATCH v2 0/4] QIOChannelFile bug fixes Ross Lagerwall
@ 2018-01-18 13:47 ` Daniel P. Berrange
0 siblings, 0 replies; 16+ messages in thread
From: Daniel P. Berrange @ 2018-01-18 13:47 UTC (permalink / raw)
To: Ross Lagerwall; +Cc: qemu-devel
On Thu, Jan 18, 2018 at 01:40:30PM +0000, Ross Lagerwall wrote:
> On 11/01/2017 02:25 PM, Ross Lagerwall wrote:
> > Hi,
> >
> > Here is a bug fix with the use of QIOChannelFile and 2 bug fixes and an
> > improvement to implementation of QIOChannelFile.
> >
> > Regards,
> > Ross Lagerwall
> >
> > Ross Lagerwall (4):
> > migration: Don't leak IO channels
> > io: Fix QIOChannelFile when creating and opening read-write
> > io: Don't call close multiple times in QIOChannelFile
> > io: Add /dev/fdset/ support to QIOChannelFile
> >
> > include/io/channel-file.h | 2 +-
> > io/channel-file.c | 11 ++++-------
> > migration/savevm.c | 2 ++
> > tests/test-io-channel-file.c | 29 +++++++++++++++++++++++++----
> > 4 files changed, 32 insertions(+), 12 deletions(-)
> >
>
> Ping for reviews...
>
> v1:
> Got feedback from Daniel P. Berrange and Marc-André Lureau.
>
> v2:
> Patch 1: Unreviewed
> Patch 2: Unreviewed
> Patch 3: Reviewed by Marc-André Lureau
> Patch 4: Reviewed by Marc-André Lureau
>
> The patch series still applies cleanly on top of master.
Sorry my bad for forgetting this. I'll review it now.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/4] io: Fix QIOChannelFile when creating and opening read-write
2017-11-01 14:25 ` [Qemu-devel] [PATCH v2 2/4] io: Fix QIOChannelFile when creating and opening read-write Ross Lagerwall
@ 2018-01-18 13:49 ` Daniel P. Berrange
0 siblings, 0 replies; 16+ messages in thread
From: Daniel P. Berrange @ 2018-01-18 13:49 UTC (permalink / raw)
To: Ross Lagerwall; +Cc: qemu-devel
On Wed, Nov 01, 2017 at 02:25:24PM +0000, Ross Lagerwall wrote:
> The code wrongly passes the mode to open() only if O_WRONLY is set.
> Instead, the mode should be passed when O_CREAT is set (or O_TMPFILE on
> Linux). Fix this by always passing the mode since open() will correctly
> ignore the mode if it is not needed. Add a testcase which exercises this
> bug and also change the existing testcase to check that the mode of the
> created file is correct.
>
> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
> ---
> Changed in v2:
> * Separated from qemu_open() change.
>
> include/io/channel-file.h | 2 +-
> io/channel-file.c | 6 +-----
> tests/test-io-channel-file.c | 29 +++++++++++++++++++++++++----
> 3 files changed, 27 insertions(+), 10 deletions(-)
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/4] io: Don't call close multiple times in QIOChannelFile
2017-11-01 14:25 ` [Qemu-devel] [PATCH v2 3/4] io: Don't call close multiple times in QIOChannelFile Ross Lagerwall
2017-11-01 18:11 ` Marc-André Lureau
@ 2018-01-18 13:50 ` Daniel P. Berrange
1 sibling, 0 replies; 16+ messages in thread
From: Daniel P. Berrange @ 2018-01-18 13:50 UTC (permalink / raw)
To: Ross Lagerwall; +Cc: qemu-devel
On Wed, Nov 01, 2017 at 02:25:25PM +0000, Ross Lagerwall wrote:
> If the file descriptor underlying QIOChannelFile is closed in the
> io_close() method, don't close it again in the finalize() method since
> the file descriptor number may have been reused in the meantime.
>
> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
> ---
> New in v2.
>
> io/channel-file.c | 1 +
> 1 file changed, 1 insertion(+)
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH v2 4/4] io: Add /dev/fdset/ support to QIOChannelFile
2017-11-01 14:25 ` [Qemu-devel] [PATCH v2 4/4] io: Add /dev/fdset/ support to QIOChannelFile Ross Lagerwall
2017-11-01 18:12 ` Marc-André Lureau
@ 2018-01-18 13:50 ` Daniel P. Berrange
1 sibling, 0 replies; 16+ messages in thread
From: Daniel P. Berrange @ 2018-01-18 13:50 UTC (permalink / raw)
To: Ross Lagerwall; +Cc: qemu-devel
On Wed, Nov 01, 2017 at 02:25:26PM +0000, Ross Lagerwall wrote:
> Add /dev/fdset/ support to QIOChannelFile by calling qemu_open() instead
> of open() and qemu_close() instead of close(). There is a subtle
> semantic change since qemu_open() automatically sets O_CLOEXEC, but this
> doesn't affect any of the users of the function.
>
> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
> ---
> Changed in v2:
> * Split into separate patch.
>
> io/channel-file.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/4] QIOChannelFile bug fixes
2017-11-01 14:25 [Qemu-devel] [PATCH v2 0/4] QIOChannelFile bug fixes Ross Lagerwall
` (4 preceding siblings ...)
2018-01-18 13:40 ` [Qemu-devel] [PATCH v2 0/4] QIOChannelFile bug fixes Ross Lagerwall
@ 2018-01-18 13:51 ` Daniel P. Berrange
5 siblings, 0 replies; 16+ messages in thread
From: Daniel P. Berrange @ 2018-01-18 13:51 UTC (permalink / raw)
To: Ross Lagerwall; +Cc: qemu-devel
On Wed, Nov 01, 2017 at 02:25:22PM +0000, Ross Lagerwall wrote:
> Hi,
>
> Here is a bug fix with the use of QIOChannelFile and 2 bug fixes and an
> improvement to implementation of QIOChannelFile.
>
> Regards,
> Ross Lagerwall
>
> Ross Lagerwall (4):
> migration: Don't leak IO channels
> io: Fix QIOChannelFile when creating and opening read-write
> io: Don't call close multiple times in QIOChannelFile
> io: Add /dev/fdset/ support to QIOChannelFile
I've queued the 3 io patches, but will leave the migration one for
the migration maintainer's queue.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/4] migration: Don't leak IO channels
2017-11-01 14:25 ` [Qemu-devel] [PATCH v2 1/4] migration: Don't leak IO channels Ross Lagerwall
2018-01-18 13:41 ` Ross Lagerwall
2018-01-18 13:44 ` Daniel P. Berrange
@ 2018-02-06 11:17 ` Dr. David Alan Gilbert
2 siblings, 0 replies; 16+ messages in thread
From: Dr. David Alan Gilbert @ 2018-02-06 11:17 UTC (permalink / raw)
To: Ross Lagerwall; +Cc: qemu-devel, Juan Quintela
* Ross Lagerwall (ross.lagerwall@citrix.com) wrote:
> Since qemu_fopen_channel_{in,out}put take references on the underlying
> IO channels, make sure to release our references to them.
>
> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Queued for migration.
Dave
> ---
> New in v2.
>
> migration/savevm.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 4a88228..87557dd 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -2259,6 +2259,7 @@ void qmp_xen_save_devices_state(const char *filename, Error **errp)
> }
> qio_channel_set_name(QIO_CHANNEL(ioc), "migration-xen-save-state");
> f = qemu_fopen_channel_output(QIO_CHANNEL(ioc));
> + object_unref(OBJECT(ioc));
> ret = qemu_save_device_state(f);
> qemu_fclose(f);
> if (ret < 0) {
> @@ -2292,6 +2293,7 @@ void qmp_xen_load_devices_state(const char *filename, Error **errp)
> }
> qio_channel_set_name(QIO_CHANNEL(ioc), "migration-xen-load-state");
> f = qemu_fopen_channel_input(QIO_CHANNEL(ioc));
> + object_unref(OBJECT(ioc));
>
> ret = qemu_loadvm_state(f);
> qemu_fclose(f);
> --
> 2.9.5
>
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2018-02-06 11:17 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-01 14:25 [Qemu-devel] [PATCH v2 0/4] QIOChannelFile bug fixes Ross Lagerwall
2017-11-01 14:25 ` [Qemu-devel] [PATCH v2 1/4] migration: Don't leak IO channels Ross Lagerwall
2018-01-18 13:41 ` Ross Lagerwall
2018-01-18 13:44 ` Daniel P. Berrange
2018-02-06 11:17 ` Dr. David Alan Gilbert
2017-11-01 14:25 ` [Qemu-devel] [PATCH v2 2/4] io: Fix QIOChannelFile when creating and opening read-write Ross Lagerwall
2018-01-18 13:49 ` Daniel P. Berrange
2017-11-01 14:25 ` [Qemu-devel] [PATCH v2 3/4] io: Don't call close multiple times in QIOChannelFile Ross Lagerwall
2017-11-01 18:11 ` Marc-André Lureau
2018-01-18 13:50 ` Daniel P. Berrange
2017-11-01 14:25 ` [Qemu-devel] [PATCH v2 4/4] io: Add /dev/fdset/ support to QIOChannelFile Ross Lagerwall
2017-11-01 18:12 ` Marc-André Lureau
2018-01-18 13:50 ` Daniel P. Berrange
2018-01-18 13:40 ` [Qemu-devel] [PATCH v2 0/4] QIOChannelFile bug fixes Ross Lagerwall
2018-01-18 13:47 ` Daniel P. Berrange
2018-01-18 13:51 ` Daniel P. Berrange
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.