|
From: | Anthony Liguori |
Subject: | [Qemu-devel] Re: [PATCH 1/10] Refactor QEMUFile for live migration |
Date: | Wed, 10 Sep 2008 10:16:55 -0500 |
User-agent: | Thunderbird 2.0.0.16 (X11/20080723) |
+static int fd_put_buffer(void *opaque, const uint8_t *buf, + int64_t pos, int size) +{ + QEMUFileFD *s = opaque; + ssize_t len; + + do { + len = write(s->fd, buf, size); + } while (len == -1 && errno == EINTR);What about the len == size case ?
I don't follow what your concern is.
+ +static int fd_close(void *opaque) +{ + QEMUFileFD *s = opaque; + qemu_free(s); + return 0; +}Why don't we need to do any specific callback for closing the file descriptor? In the case of a socket, I imagine we may want to shut the socket down, for ex.
Since qemu_fopen_fd takes a previously open file descriptor, the expectation is that you're going to be able to close it yourself at some point. This worked out fine for the migration code and I think it'll also work out okay for other code. Plus, you would have to add callbacks to qemu_fopen_fd() which gets pretty nasty.
+ +QEMUFile *qemu_fopen_fd(int fd) +{ + QEMUFileFD *s = qemu_mallocz(sizeof(QEMUFileFD));can't it fail?
Yeah, I should add error checking.
-static QEMUFile *qemu_fopen_bdrv(BlockDriverState *bs, int64_t offset, int is_writable) +typedef struct QEMUFileBdrv +{ + BlockDriverState *bs; + int64_t base_offset; +} QEMUFileBdrv;Isn't it possible to abstract the differences between bdrv and file so to have a common implementation between them? Do you think it's worthwhile ?
It's a lot of work. QEMUFile is optimized to batch short read/write operations whereas BlockDriverState is meant to be sector based. QEMUFile is also evolving into a stream mechanism where BlockDriver will always be random access.
It's certainly possible, but I don't think it's worth it at this stage. Thanks for the review! Regards, Anthony Liguori
[Prev in Thread] | Current Thread | [Next in Thread] |