* [Qemu-devel] [PULL 01/15] scsi: revert change to scsi_req_cancel_async and add assertions
2016-01-15 16:04 [Qemu-devel] [PULL 00/15] NBD, chardev, SCSI patches for 2015-01-15 Paolo Bonzini
@ 2016-01-15 16:04 ` Paolo Bonzini
2016-01-15 16:04 ` [Qemu-devel] [PULL 02/15] target-i386: do not duplicate page protection checks Paolo Bonzini
` (15 subsequent siblings)
16 siblings, 0 replies; 30+ messages in thread
From: Paolo Bonzini @ 2016-01-15 16:04 UTC (permalink / raw)
To: qemu-devel
Fam Zheng noticed that the change in commit 36896bf ("scsi: always call
notifier on async cancellation", 2015-12-16) could cause a leak of
the request; scsi_req_cancel_async now calls scsi_req_ref
multiple times for multiple cancellations, but there is only
one call to scsi_req_cancel_complete.
So revert the patch and instead assert that the problematic case (a call
to scsi_req_cancel_async after the aiocb has been completed) cannot
happen.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/scsi/scsi-bus.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index 00bddc9..378bf4d 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -1759,6 +1759,15 @@ void scsi_req_cancel_async(SCSIRequest *req, Notifier *notifier)
if (notifier) {
notifier_list_add(&req->cancel_notifiers, notifier);
}
+ if (req->io_canceled) {
+ /* A blk_aio_cancel_async is pending; when it finishes,
+ * scsi_req_cancel_complete will be called and will
+ * call the notifier we just added. Just wait for that.
+ */
+ assert(req->aiocb);
+ return;
+ }
+ /* Dropped in scsi_req_cancel_complete. */
scsi_req_ref(req);
scsi_req_dequeue(req);
req->io_canceled = true;
@@ -1775,6 +1784,8 @@ void scsi_req_cancel(SCSIRequest *req)
if (!req->enqueued) {
return;
}
+ assert(!req->io_canceled);
+ /* Dropped in scsi_req_cancel_complete. */
scsi_req_ref(req);
scsi_req_dequeue(req);
req->io_canceled = true;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* [Qemu-devel] [PULL 02/15] target-i386: do not duplicate page protection checks
2016-01-15 16:04 [Qemu-devel] [PULL 00/15] NBD, chardev, SCSI patches for 2015-01-15 Paolo Bonzini
2016-01-15 16:04 ` [Qemu-devel] [PULL 01/15] scsi: revert change to scsi_req_cancel_async and add assertions Paolo Bonzini
@ 2016-01-15 16:04 ` Paolo Bonzini
2016-01-15 16:04 ` [Qemu-devel] [PULL 03/15] i386: avoid null pointer dereference Paolo Bonzini
` (14 subsequent siblings)
16 siblings, 0 replies; 30+ messages in thread
From: Paolo Bonzini @ 2016-01-15 16:04 UTC (permalink / raw)
To: qemu-devel
x86_cpu_handle_mmu_fault is currently checking twice for writability
and executability of pages; the first time to decide whether to
trigger a page fault, the second time to compute the "prot" argument
to tlb_set_page_with_attrs.
Reorganize code so that first "prot" is computed, then it is used
to check whether to raise a page fault, then finally PROT_WRITE is
removed if the D bit will have to be set.
Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target-i386/helper.c | 65 +++++++++++++++++++---------------------------------
1 file changed, 23 insertions(+), 42 deletions(-)
diff --git a/target-i386/helper.c b/target-i386/helper.c
index d18be95..bf58242 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -890,38 +890,30 @@ do_check_protect_pse36:
goto do_fault_rsvd;
}
ptep ^= PG_NX_MASK;
- if ((ptep & PG_NX_MASK) && is_write1 == 2) {
+
+ /* can the page can be put in the TLB? prot will tell us */
+ if (is_user && !(ptep & PG_USER_MASK)) {
goto do_fault_protect;
}
- switch (mmu_idx) {
- case MMU_USER_IDX:
- if (!(ptep & PG_USER_MASK)) {
- goto do_fault_protect;
- }
- if (is_write && !(ptep & PG_RW_MASK)) {
- goto do_fault_protect;
- }
- break;
- case MMU_KSMAP_IDX:
- if (is_write1 != 2 && (ptep & PG_USER_MASK)) {
- goto do_fault_protect;
+ prot = 0;
+ if (mmu_idx != MMU_KSMAP_IDX || !(ptep & PG_USER_MASK)) {
+ prot |= PAGE_READ;
+ if ((ptep & PG_RW_MASK) || (!is_user && !(env->cr[0] & CR0_WP_MASK))) {
+ prot |= PAGE_WRITE;
}
- /* fall through */
- case MMU_KNOSMAP_IDX:
- if (is_write1 == 2 && (env->cr[4] & CR4_SMEP_MASK) &&
- (ptep & PG_USER_MASK)) {
- goto do_fault_protect;
- }
- if ((env->cr[0] & CR0_WP_MASK) &&
- is_write && !(ptep & PG_RW_MASK)) {
- goto do_fault_protect;
- }
- break;
+ }
+ if (!(ptep & PG_NX_MASK) &&
+ (mmu_idx == MMU_USER_IDX ||
+ !((env->cr[4] & CR4_SMEP_MASK) && (ptep & PG_USER_MASK)))) {
+ prot |= PAGE_EXEC;
+ }
- default: /* cannot happen */
- break;
+ if ((prot & (1 << is_write1)) == 0) {
+ goto do_fault_protect;
}
+
+ /* yes, it can! */
is_dirty = is_write && !(pte & PG_DIRTY_MASK);
if (!(pte & PG_ACCESSED_MASK) || is_dirty) {
pte |= PG_ACCESSED_MASK;
@@ -931,25 +923,13 @@ do_check_protect_pse36:
x86_stl_phys_notdirty(cs, pte_addr, pte);
}
- /* the page can be put in the TLB */
- prot = PAGE_READ;
- if (!(ptep & PG_NX_MASK) &&
- (mmu_idx == MMU_USER_IDX ||
- !((env->cr[4] & CR4_SMEP_MASK) && (ptep & PG_USER_MASK)))) {
- prot |= PAGE_EXEC;
- }
- if (pte & PG_DIRTY_MASK) {
+ if (!(pte & PG_DIRTY_MASK)) {
/* only set write access if already dirty... otherwise wait
for dirty access */
- if (is_user) {
- if (ptep & PG_RW_MASK)
- prot |= PAGE_WRITE;
- } else {
- if (!(env->cr[0] & CR0_WP_MASK) ||
- (ptep & PG_RW_MASK))
- prot |= PAGE_WRITE;
- }
+ assert(!is_write);
+ prot &= ~PROT_WRITE;
}
+
do_mapping:
pte = pte & env->a20_mask;
@@ -962,6 +942,7 @@ do_check_protect_pse36:
page_offset = vaddr & (page_size - 1);
paddr = pte + page_offset;
+ assert(prot & (1 << is_write1));
tlb_set_page_with_attrs(cs, vaddr, paddr, cpu_get_mem_attrs(env),
prot, mmu_idx, page_size);
return 0;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* [Qemu-devel] [PULL 03/15] i386: avoid null pointer dereference
2016-01-15 16:04 [Qemu-devel] [PULL 00/15] NBD, chardev, SCSI patches for 2015-01-15 Paolo Bonzini
2016-01-15 16:04 ` [Qemu-devel] [PULL 01/15] scsi: revert change to scsi_req_cancel_async and add assertions Paolo Bonzini
2016-01-15 16:04 ` [Qemu-devel] [PULL 02/15] target-i386: do not duplicate page protection checks Paolo Bonzini
@ 2016-01-15 16:04 ` Paolo Bonzini
2016-01-15 16:53 ` Eric Blake
2016-01-15 16:04 ` [Qemu-devel] [PULL] " Paolo Bonzini
` (13 subsequent siblings)
16 siblings, 1 reply; 30+ messages in thread
From: Paolo Bonzini @ 2016-01-15 16:04 UTC (permalink / raw)
To: qemu-devel; +Cc: P J P
From: P J P <ppandit@redhat.com>
Hello,
A null pointer dereference issue was reported by Mr Ling Liu, CC'd here. It
occurs while doing I/O port write operations via hmp interface. In that,
'current_cpu' remains null as it is not called from cpu_exec loop, which
results in the said issue.
Below is a proposed (tested)patch to fix this issue; Does it look okay?
===
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PULL 03/15] i386: avoid null pointer dereference
2016-01-15 16:04 ` [Qemu-devel] [PULL 03/15] i386: avoid null pointer dereference Paolo Bonzini
@ 2016-01-15 16:53 ` Eric Blake
2016-01-15 17:09 ` Paolo Bonzini
0 siblings, 1 reply; 30+ messages in thread
From: Eric Blake @ 2016-01-15 16:53 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel; +Cc: P J P
[-- Attachment #1: Type: text/plain, Size: 789 bytes --]
Did 'git send-email' get confused? This has no contents, and then there
is a message "[Qemu-devel] [PULL] i386: avoid null pointer dereference"
with no mention of where it fits in the series (presumably 3/15).
On 01/15/2016 09:04 AM, Paolo Bonzini wrote:
> From: P J P <ppandit@redhat.com>
>
> Hello,
>
> A null pointer dereference issue was reported by Mr Ling Liu, CC'd here. It
> occurs while doing I/O port write operations via hmp interface. In that,
> 'current_cpu' remains null as it is not called from cpu_exec loop, which
> results in the said issue.
>
> Below is a proposed (tested)patch to fix this issue; Does it look okay?
>
> ===
>
>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PULL 03/15] i386: avoid null pointer dereference
2016-01-15 16:53 ` Eric Blake
@ 2016-01-15 17:09 ` Paolo Bonzini
2016-01-15 19:46 ` P J P
0 siblings, 1 reply; 30+ messages in thread
From: Paolo Bonzini @ 2016-01-15 17:09 UTC (permalink / raw)
To: Eric Blake, qemu-devel; +Cc: P J P
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
On 15/01/2016 17:53, Eric Blake wrote:
> Did 'git send-email' get confused? This has no contents, and then
> there is a message "[Qemu-devel] [PULL] i386: avoid null pointer
> dereference" with no mention of where it fits in the series
> (presumably 3/15).
This is probably due to the original patch being formatted badly, and
my workload assuming that a patch series or pull request can be
manipulated as a single mbox file.
Paolo
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2
iQEcBAEBCAAGBQJWmSe5AAoJEL/70l94x66DUiUH/18ishjBK0cp8gln3VY6Qiih
akKIcQ5I4tq5/U9ChqQPyc1OgfF/ocYIZh0iJLuNn9PKxEm59tIhHC+ZZIvZI16V
CdWZDhJqyYUhu/Vz34UzhWbtXdZG8mVJgTj8eBHCd62V9eMMySKQ1wQg7C6FnDqe
RuzsE7U/N/ZKCGbkZgzrU685u2uWd1v+dOZ2kiODzvBqA3hqVb6phJ1PxsPTm22E
Erc7VE8nuNQDByeUKhWx4gHE67e8OZTBOK9dJ3bUgk7hP1caSzmjNUBrP2eqxFPx
Dku+/92UJ4yHXu/LZqW0yLXB50Z0cObw50oX5jvdZ3ZSWlp+pNm3GI3rduPmvi4=
=SoPv
-----END PGP SIGNATURE-----
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [PULL] i386: avoid null pointer dereference
2016-01-15 16:04 [Qemu-devel] [PULL 00/15] NBD, chardev, SCSI patches for 2015-01-15 Paolo Bonzini
` (2 preceding siblings ...)
2016-01-15 16:04 ` [Qemu-devel] [PULL 03/15] i386: avoid null pointer dereference Paolo Bonzini
@ 2016-01-15 16:04 ` Paolo Bonzini
2016-01-15 16:04 ` [Qemu-devel] [PULL 04/15] scsi: initialise info object with appropriate size Paolo Bonzini
` (12 subsequent siblings)
16 siblings, 0 replies; 30+ messages in thread
From: Paolo Bonzini @ 2016-01-15 16:04 UTC (permalink / raw)
To: qemu-devel; +Cc: Prasad J Pandit
From: Prasad J Pandit <pjp@fedoraproject.org>
When I/O port write operation is called from hmp interface,
'current_cpu' remains null, as it is not called from cpu_exec()
loop. This leads to a null pointer dereference in vapic_write
routine. Add check to avoid it.
Reported-by: Ling Liu <liuling-it@360.cn>
Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
Message-Id: <alpine.LFD.2.20.1512181129320.9805@wniryva>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/i386/kvmvapic.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c
index c6d34b2..f0922da 100644
--- a/hw/i386/kvmvapic.c
+++ b/hw/i386/kvmvapic.c
@@ -634,13 +634,18 @@ static int vapic_prepare(VAPICROMState *s)
static void vapic_write(void *opaque, hwaddr addr, uint64_t data,
unsigned int size)
{
- CPUState *cs = current_cpu;
- X86CPU *cpu = X86_CPU(cs);
- CPUX86State *env = &cpu->env;
- hwaddr rom_paddr;
VAPICROMState *s = opaque;
+ X86CPU *cpu;
+ CPUX86State *env;
+ hwaddr rom_paddr;
- cpu_synchronize_state(cs);
+ if (!current_cpu) {
+ return;
+ }
+
+ cpu_synchronize_state(current_cpu);
+ cpu = X86_CPU(current_cpu);
+ env = &cpu->env;
/*
* The VAPIC supports two PIO-based hypercalls, both via port 0x7E.
--
1.8.3.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* [Qemu-devel] [PULL 04/15] scsi: initialise info object with appropriate size
2016-01-15 16:04 [Qemu-devel] [PULL 00/15] NBD, chardev, SCSI patches for 2015-01-15 Paolo Bonzini
` (3 preceding siblings ...)
2016-01-15 16:04 ` [Qemu-devel] [PULL] " Paolo Bonzini
@ 2016-01-15 16:04 ` Paolo Bonzini
2016-01-15 16:04 ` [Qemu-devel] [PULL 05/15] vmw_pvscsi: x-disable-pcie, x-old-pci-configuration back-compat props are 2.5 specific Paolo Bonzini
` (11 subsequent siblings)
16 siblings, 0 replies; 30+ messages in thread
From: Paolo Bonzini @ 2016-01-15 16:04 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-stable, Prasad J Pandit, P J P
From: P J P <ppandit@redhat.com>
While processing controller 'CTRL_GET_INFO' command, the routine
'megasas_ctrl_get_info' overflows the '&info' object size. Use its
appropriate size to null initialise it.
Reported-by: Qinghao Tang <luodalongde@gmail.com>
Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
Message-Id: <alpine.LFD.2.20.1512211501420.22471@wniryva>
Cc: qemu-stable@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/scsi/megasas.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index d7dc667..576f56c 100644
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -718,7 +718,7 @@ static int megasas_ctrl_get_info(MegasasState *s, MegasasCmd *cmd)
BusChild *kid;
int num_pd_disks = 0;
- memset(&info, 0x0, cmd->iov_size);
+ memset(&info, 0x0, dcmd_size);
if (cmd->iov_size < dcmd_size) {
trace_megasas_dcmd_invalid_xfer_len(cmd->index, cmd->iov_size,
dcmd_size);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* [Qemu-devel] [PULL 05/15] vmw_pvscsi: x-disable-pcie, x-old-pci-configuration back-compat props are 2.5 specific
2016-01-15 16:04 [Qemu-devel] [PULL 00/15] NBD, chardev, SCSI patches for 2015-01-15 Paolo Bonzini
` (4 preceding siblings ...)
2016-01-15 16:04 ` [Qemu-devel] [PULL 04/15] scsi: initialise info object with appropriate size Paolo Bonzini
@ 2016-01-15 16:04 ` Paolo Bonzini
2016-01-15 16:04 ` [Qemu-devel] [PULL 06/15] qemu-char: delete send_all/recv_all helper methods Paolo Bonzini
` (10 subsequent siblings)
16 siblings, 0 replies; 30+ messages in thread
From: Paolo Bonzini @ 2016-01-15 16:04 UTC (permalink / raw)
To: qemu-devel; +Cc: Shmulik Ladkani
From: Shmulik Ladkani <shmulik.ladkani@ravellosystems.com>
pvscsi's x-disable-pcie and x-old-pci-configuration backward compat
properties were introduced in 952970b and d5da3ef:
vmw_pvscsi: Introduce 'x-old-pci-configuration' backword compatability property
vmw_pvscsi: Introduce 'x-disable-pcie' backword compatability property
and were placed into HW_COMPAT_2_4.
However since these commits were pulled post v2.5, move them to
HW_COMPAT_2_5.
Signed-off-by: Shmulik Ladkani <shmulik.ladkani@ravellosystems.com>
Message-Id: <1450900558-20113-1-git-send-email-shmulik.ladkani@ravellosystems.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
include/hw/compat.h | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/include/hw/compat.h b/include/hw/compat.h
index 98df0dd..491884b 100644
--- a/include/hw/compat.h
+++ b/include/hw/compat.h
@@ -3,6 +3,15 @@
#define HW_COMPAT_2_5 \
{\
+ .driver = "pvscsi",\
+ .property = "x-old-pci-configuration",\
+ .value = "on",\
+ },{\
+ .driver = "pvscsi",\
+ .property = "x-disable-pcie",\
+ .value = "on",\
+ },\
+ {\
.driver = "vmxnet3",\
.property = "x-old-msi-offsets",\
.value = "on",\
@@ -18,14 +27,6 @@
.property = "scsi",\
.value = "true",\
},{\
- .driver = "pvscsi",\
- .property = "x-old-pci-configuration",\
- .value = "on",\
- },{\
- .driver = "pvscsi",\
- .property = "x-disable-pcie",\
- .value = "on",\
- },{\
.driver = "e1000",\
.property = "extra_mac_registers",\
.value = "off",\
--
1.8.3.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* [Qemu-devel] [PULL 06/15] qemu-char: delete send_all/recv_all helper methods
2016-01-15 16:04 [Qemu-devel] [PULL 00/15] NBD, chardev, SCSI patches for 2015-01-15 Paolo Bonzini
` (5 preceding siblings ...)
2016-01-15 16:04 ` [Qemu-devel] [PULL 05/15] vmw_pvscsi: x-disable-pcie, x-old-pci-configuration back-compat props are 2.5 specific Paolo Bonzini
@ 2016-01-15 16:04 ` Paolo Bonzini
2016-01-15 16:04 ` [Qemu-devel] [PULL 07/15] iscsi: send readcapacity10 when readcapacity16 failed Paolo Bonzini
` (9 subsequent siblings)
16 siblings, 0 replies; 30+ messages in thread
From: Paolo Bonzini @ 2016-01-15 16:04 UTC (permalink / raw)
To: qemu-devel
From: "Daniel P. Berrange" <berrange@redhat.com>
The qemu-char.c contains two helper methods send_all
and recv_all. These are in fact declared in sockets.h
so ought to have been in util/qemu-sockets.c. For added
fun the impl of recv_all is completely missing on Win32.
Fortunately there is only a single caller of these
methods, the TPM passthrough code, which is only
ever compiled on Linux. With only a single caller
these helpers are not compelling enough to keep so
inline them in the TPM code, avoiding the need to
fix the missing recv_all on Win32.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-Id: <1450879144-17111-1-git-send-email-berrange@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/tpm/tpm_passthrough.c | 29 ++++++++++++++++++--
include/qemu/sockets.h | 2 --
qemu-char.c | 71 ------------------------------------------------
3 files changed, 27 insertions(+), 75 deletions(-)
diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
index be160c1..ab526db 100644
--- a/hw/tpm/tpm_passthrough.c
+++ b/hw/tpm/tpm_passthrough.c
@@ -83,12 +83,37 @@ static void tpm_passthrough_cancel_cmd(TPMBackend *tb);
static int tpm_passthrough_unix_write(int fd, const uint8_t *buf, uint32_t len)
{
- return send_all(fd, buf, len);
+ int ret, remain;
+
+ remain = len;
+ while (len > 0) {
+ ret = write(fd, buf, remain);
+ if (ret < 0) {
+ if (errno != EINTR && errno != EAGAIN) {
+ return -1;
+ }
+ } else if (ret == 0) {
+ break;
+ } else {
+ buf += ret;
+ remain -= ret;
+ }
+ }
+ return len - remain;
}
static int tpm_passthrough_unix_read(int fd, uint8_t *buf, uint32_t len)
{
- return recv_all(fd, buf, len, true);
+ int ret;
+ reread:
+ ret = read(fd, buf, len);
+ if (ret < 0) {
+ if (errno != EINTR && errno != EAGAIN) {
+ return -1;
+ }
+ goto reread;
+ }
+ return ret;
}
static uint32_t tpm_passthrough_get_size_from_buffer(const uint8_t *buf)
diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
index 2e7f985..c1a81fa 100644
--- a/include/qemu/sockets.h
+++ b/include/qemu/sockets.h
@@ -40,8 +40,6 @@ int socket_set_nodelay(int fd);
void qemu_set_block(int fd);
void qemu_set_nonblock(int fd);
int socket_set_fast_reuse(int fd);
-int send_all(int fd, const void *buf, int len1);
-int recv_all(int fd, void *buf, int len1, bool single_read);
#ifdef WIN32
/* Windows has different names for the same constants with the same values */
diff --git a/qemu-char.c b/qemu-char.c
index 00a7526..d7be185 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -696,77 +696,6 @@ static CharDriverState *qemu_chr_open_mux(const char *id,
}
-#ifdef _WIN32
-int send_all(int fd, const void *buf, int len1)
-{
- int ret, len;
-
- len = len1;
- while (len > 0) {
- ret = send(fd, buf, len, 0);
- if (ret < 0) {
- errno = WSAGetLastError();
- if (errno != WSAEWOULDBLOCK) {
- return -1;
- }
- } else if (ret == 0) {
- break;
- } else {
- buf += ret;
- len -= ret;
- }
- }
- return len1 - len;
-}
-
-#else
-
-int send_all(int fd, const void *_buf, int len1)
-{
- int ret, len;
- const uint8_t *buf = _buf;
-
- len = len1;
- while (len > 0) {
- ret = write(fd, buf, len);
- if (ret < 0) {
- if (errno != EINTR && errno != EAGAIN)
- return -1;
- } else if (ret == 0) {
- break;
- } else {
- buf += ret;
- len -= ret;
- }
- }
- return len1 - len;
-}
-
-int recv_all(int fd, void *_buf, int len1, bool single_read)
-{
- int ret, len;
- uint8_t *buf = _buf;
-
- len = len1;
- while ((len > 0) && (ret = read(fd, buf, len)) != 0) {
- if (ret < 0) {
- if (errno != EINTR && errno != EAGAIN) {
- return -1;
- }
- continue;
- } else {
- if (single_read) {
- return ret;
- }
- buf += ret;
- len -= ret;
- }
- }
- return len1 - len;
-}
-
-#endif /* !_WIN32 */
-
typedef struct IOWatchPoll
{
GSource parent;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* [Qemu-devel] [PULL 07/15] iscsi: send readcapacity10 when readcapacity16 failed
2016-01-15 16:04 [Qemu-devel] [PULL 00/15] NBD, chardev, SCSI patches for 2015-01-15 Paolo Bonzini
` (6 preceding siblings ...)
2016-01-15 16:04 ` [Qemu-devel] [PULL 06/15] qemu-char: delete send_all/recv_all helper methods Paolo Bonzini
@ 2016-01-15 16:04 ` Paolo Bonzini
2016-01-15 16:04 ` [Qemu-devel] [PULL 08/15] SCSI device: fix to incomplete QOMify Paolo Bonzini
` (8 subsequent siblings)
16 siblings, 0 replies; 30+ messages in thread
From: Paolo Bonzini @ 2016-01-15 16:04 UTC (permalink / raw)
To: qemu-devel; +Cc: Zhu Lingshan
From: Zhu Lingshan <lszhu@suse.com>
When play with Dell MD3000 target, for sure it
is a TYPE_DISK, but readcapacity16 would fail.
Then we find that readcapacity10 succeeded. It
looks like the target just support readcapacity10
even through it is a TYPE_DISK or have some
TYPE_ROM characteristics.
This patch can give a chance to send
readcapacity16 when readcapacity10 failed.
This patch is not harmful to original pathes
Signed-off-by: Zhu Lingshan <lszhu@suse.com>
Message-Id: <1451359934-9236-1-git-send-email-lszhu@suse.com>
[Don't fall through on UNIT ATTENTION. - Paolo]
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
block/iscsi.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/block/iscsi.c b/block/iscsi.c
index eb28ddc..3acb052 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1243,8 +1243,13 @@ static void iscsi_readcapacity_sync(IscsiLun *iscsilun, Error **errp)
iscsilun->lbprz = !!rc16->lbprz;
iscsilun->use_16_for_rw = (rc16->returned_lba > 0xffffffff);
}
+ break;
}
- break;
+ if (task != NULL && task->status == SCSI_STATUS_CHECK_CONDITION
+ && task->sense.key == SCSI_SENSE_UNIT_ATTENTION) {
+ break;
+ }
+ /* Fall through and try READ CAPACITY(10) instead. */
case TYPE_ROM:
task = iscsi_readcapacity10_sync(iscsilun->iscsi, iscsilun->lun, 0, 0);
if (task != NULL && task->status == SCSI_STATUS_GOOD) {
--
1.8.3.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* [Qemu-devel] [PULL 08/15] SCSI device: fix to incomplete QOMify
2016-01-15 16:04 [Qemu-devel] [PULL 00/15] NBD, chardev, SCSI patches for 2015-01-15 Paolo Bonzini
` (7 preceding siblings ...)
2016-01-15 16:04 ` [Qemu-devel] [PULL 07/15] iscsi: send readcapacity10 when readcapacity16 failed Paolo Bonzini
@ 2016-01-15 16:04 ` Paolo Bonzini
2016-01-15 16:04 ` [Qemu-devel] [PULL 09/15] nbd: Always call "close_fn" in nbd_client_new Paolo Bonzini
` (7 subsequent siblings)
16 siblings, 0 replies; 30+ messages in thread
From: Paolo Bonzini @ 2016-01-15 16:04 UTC (permalink / raw)
To: qemu-devel; +Cc: Cao jin
From: Cao jin <caoj.fnst@cn.fujitsu.com>
Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Message-Id: <1452073066-28319-1-git-send-email-caoj.fnst@cn.fujitsu.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/scsi/megasas.c | 12 ++++++------
hw/scsi/scsi-bus.c | 4 ++--
hw/scsi/virtio-scsi.c | 2 +-
3 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index 576f56c..60c0e6c 100644
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -744,7 +744,7 @@ static int megasas_ctrl_get_info(MegasasState *s, MegasasCmd *cmd)
info.device.type = MFI_INFO_DEV_SAS3G;
info.device.port_count = 8;
QTAILQ_FOREACH(kid, &s->bus.qbus.children, sibling) {
- SCSIDevice *sdev = DO_UPCAST(SCSIDevice, qdev, kid->child);
+ SCSIDevice *sdev = SCSI_DEVICE(kid->child);
uint16_t pd_id;
if (num_pd_disks < 8) {
@@ -960,7 +960,7 @@ static int megasas_dcmd_pd_get_list(MegasasState *s, MegasasCmd *cmd)
max_pd_disks = MFI_MAX_SYS_PDS;
}
QTAILQ_FOREACH(kid, &s->bus.qbus.children, sibling) {
- SCSIDevice *sdev = DO_UPCAST(SCSIDevice, qdev, kid->child);
+ SCSIDevice *sdev = SCSI_DEVICE(kid->child);
uint16_t pd_id;
if (num_pd_disks >= max_pd_disks)
@@ -1136,7 +1136,7 @@ static int megasas_dcmd_ld_get_list(MegasasState *s, MegasasCmd *cmd)
max_ld_disks = MFI_MAX_LD;
}
QTAILQ_FOREACH(kid, &s->bus.qbus.children, sibling) {
- SCSIDevice *sdev = DO_UPCAST(SCSIDevice, qdev, kid->child);
+ SCSIDevice *sdev = SCSI_DEVICE(kid->child);
if (num_ld_disks >= max_ld_disks) {
break;
@@ -1187,7 +1187,7 @@ static int megasas_dcmd_ld_list_query(MegasasState *s, MegasasCmd *cmd)
max_ld_disks = MFI_MAX_LD;
}
QTAILQ_FOREACH(kid, &s->bus.qbus.children, sibling) {
- SCSIDevice *sdev = DO_UPCAST(SCSIDevice, qdev, kid->child);
+ SCSIDevice *sdev = SCSI_DEVICE(kid->child);
if (num_ld_disks >= max_ld_disks) {
break;
@@ -1327,7 +1327,7 @@ static int megasas_dcmd_cfg_read(MegasasState *s, MegasasCmd *cmd)
ld_offset = array_offset + sizeof(struct mfi_array) * num_pd_disks;
QTAILQ_FOREACH(kid, &s->bus.qbus.children, sibling) {
- SCSIDevice *sdev = DO_UPCAST(SCSIDevice, qdev, kid->child);
+ SCSIDevice *sdev = SCSI_DEVICE(kid->child);
uint16_t sdev_id = ((sdev->id & 0xFF) << 8) | (sdev->lun & 0xFF);
struct mfi_array *array;
struct mfi_ld_config *ld;
@@ -2237,7 +2237,7 @@ static void megasas_soft_reset(MegasasState *s)
* after the initial reset.
*/
QTAILQ_FOREACH(kid, &s->bus.qbus.children, sibling) {
- SCSIDevice *sdev = DO_UPCAST(SCSIDevice, qdev, kid->child);
+ SCSIDevice *sdev = SCSI_DEVICE(kid->child);
sdev->unit_attention = SENSE_CODE(NO_SENSE);
scsi_device_unit_attention_reported(sdev);
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index 378bf4d..164af87 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -1861,7 +1861,7 @@ void scsi_device_purge_requests(SCSIDevice *sdev, SCSISense sense)
static char *scsibus_get_dev_path(DeviceState *dev)
{
- SCSIDevice *d = DO_UPCAST(SCSIDevice, qdev, dev);
+ SCSIDevice *d = SCSI_DEVICE(dev);
DeviceState *hba = dev->parent_bus->parent;
char *id;
char *path;
@@ -2034,7 +2034,7 @@ static void scsi_device_class_init(ObjectClass *klass, void *data)
static void scsi_dev_instance_init(Object *obj)
{
DeviceState *dev = DEVICE(obj);
- SCSIDevice *s = DO_UPCAST(SCSIDevice, qdev, dev);
+ SCSIDevice *s = SCSI_DEVICE(dev);
device_add_bootindex_property(obj, &s->conf.bootindex,
"bootindex", NULL,
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 3a4f520..607593c 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -352,7 +352,7 @@ static int virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req)
target = req->req.tmf.lun[1];
s->resetting++;
QTAILQ_FOREACH(kid, &s->bus.qbus.children, sibling) {
- d = DO_UPCAST(SCSIDevice, qdev, kid->child);
+ d = SCSI_DEVICE(kid->child);
if (d->channel == 0 && d->id == target) {
qdev_reset_all(&d->qdev);
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* [Qemu-devel] [PULL 09/15] nbd: Always call "close_fn" in nbd_client_new
2016-01-15 16:04 [Qemu-devel] [PULL 00/15] NBD, chardev, SCSI patches for 2015-01-15 Paolo Bonzini
` (8 preceding siblings ...)
2016-01-15 16:04 ` [Qemu-devel] [PULL 08/15] SCSI device: fix to incomplete QOMify Paolo Bonzini
@ 2016-01-15 16:04 ` Paolo Bonzini
2016-01-15 16:04 ` [Qemu-devel] [PULL 10/15] nbd: Split nbd.c Paolo Bonzini
` (6 subsequent siblings)
16 siblings, 0 replies; 30+ messages in thread
From: Paolo Bonzini @ 2016-01-15 16:04 UTC (permalink / raw)
To: qemu-devel; +Cc: Fam Zheng
From: Fam Zheng <famz@redhat.com>
Rename the parameter "close" to "close_fn" to disambiguous with
close(2).
This unifies error handling paths of NBDClient allocation:
nbd_client_new will shutdown the socket and call the "close_fn" callback
if negotiation failed, so the caller don't need a different path than
the normal close.
The returned pointer is never used, make it void in preparation for the
next patch.
Signed-off-by: Fam Zheng <famz@redhat.com>
Message-Id: <1452760863-25350-2-git-send-email-famz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
blockdev-nbd.c | 5 ++---
include/block/nbd.h | 3 +--
nbd.c | 11 +++++------
qemu-nbd.c | 10 +++-------
4 files changed, 11 insertions(+), 18 deletions(-)
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index bcdd18b..4a758ac 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -27,9 +27,8 @@ static void nbd_accept(void *opaque)
socklen_t addr_len = sizeof(addr);
int fd = accept(server_fd, (struct sockaddr *)&addr, &addr_len);
- if (fd >= 0 && !nbd_client_new(NULL, fd, nbd_client_put)) {
- shutdown(fd, 2);
- close(fd);
+ if (fd >= 0) {
+ nbd_client_new(NULL, fd, nbd_client_put);
}
}
diff --git a/include/block/nbd.h b/include/block/nbd.h
index 65f409d..7eccb41 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -98,8 +98,7 @@ NBDExport *nbd_export_find(const char *name);
void nbd_export_set_name(NBDExport *exp, const char *name);
void nbd_export_close_all(void);
-NBDClient *nbd_client_new(NBDExport *exp, int csock,
- void (*close)(NBDClient *));
+void nbd_client_new(NBDExport *exp, int csock, void (*close_fn)(NBDClient *));
void nbd_client_get(NBDClient *client);
void nbd_client_put(NBDClient *client);
diff --git a/nbd.c b/nbd.c
index b3d9654..f8d0221 100644
--- a/nbd.c
+++ b/nbd.c
@@ -1475,8 +1475,7 @@ static void nbd_update_can_read(NBDClient *client)
}
}
-NBDClient *nbd_client_new(NBDExport *exp, int csock,
- void (*close)(NBDClient *))
+void nbd_client_new(NBDExport *exp, int csock, void (*close_fn)(NBDClient *))
{
NBDClient *client;
client = g_malloc0(sizeof(NBDClient));
@@ -1485,10 +1484,11 @@ NBDClient *nbd_client_new(NBDExport *exp, int csock,
client->sock = csock;
client->can_read = true;
if (nbd_send_negotiate(client)) {
- g_free(client);
- return NULL;
+ shutdown(client->sock, 2);
+ close_fn(client);
+ return;
}
- client->close = close;
+ client->close = close_fn;
qemu_co_mutex_init(&client->send_lock);
nbd_set_handlers(client);
@@ -1496,5 +1496,4 @@ NBDClient *nbd_client_new(NBDExport *exp, int csock,
QTAILQ_INSERT_TAIL(&exp->clients, client, next);
nbd_export_get(exp);
}
- return client;
}
diff --git a/qemu-nbd.c b/qemu-nbd.c
index a4cf847..ede4a54 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -333,13 +333,9 @@ static void nbd_accept(void *opaque)
return;
}
- if (nbd_client_new(exp, fd, nbd_client_closed)) {
- nb_fds++;
- nbd_update_server_fd_handler(server_fd);
- } else {
- shutdown(fd, 2);
- close(fd);
- }
+ nb_fds++;
+ nbd_update_server_fd_handler(server_fd);
+ nbd_client_new(exp, fd, nbd_client_closed);
}
static void nbd_update_server_fd_handler(int fd)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* [Qemu-devel] [PULL 10/15] nbd: Split nbd.c
2016-01-15 16:04 [Qemu-devel] [PULL 00/15] NBD, chardev, SCSI patches for 2015-01-15 Paolo Bonzini
` (9 preceding siblings ...)
2016-01-15 16:04 ` [Qemu-devel] [PULL 09/15] nbd: Always call "close_fn" in nbd_client_new Paolo Bonzini
@ 2016-01-15 16:04 ` Paolo Bonzini
2016-01-15 16:04 ` [Qemu-devel] [PULL 11/15] nbd-server: Coroutine based negotiation Paolo Bonzini
` (5 subsequent siblings)
16 siblings, 0 replies; 30+ messages in thread
From: Paolo Bonzini @ 2016-01-15 16:04 UTC (permalink / raw)
To: qemu-devel; +Cc: Fam Zheng
From: Fam Zheng <famz@redhat.com>
We have NBD server code and client code, all mixed in a file. Now split
them into separate files under nbd/, and update MAINTAINERS.
filter_nbd for iotest 083 is updated to keep the log filtered out.
Signed-off-by: Fam Zheng <famz@redhat.com>
Message-Id: <1452760863-25350-3-git-send-email-famz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
MAINTAINERS | 5 +-
Makefile.objs | 3 +-
nbd/Makefile.objs | 1 +
nbd/client.c | 361 +++++++++++++++++++++++++++++++++++++++
nbd/common.c | 64 +++++++
nbd/nbd-internal.h | 113 +++++++++++++
nbd.c => nbd/server.c | 451 +------------------------------------------------
tests/qemu-iotests/083 | 2 +-
8 files changed, 547 insertions(+), 453 deletions(-)
create mode 100644 nbd/Makefile.objs
create mode 100644 nbd/client.c
create mode 100644 nbd/common.c
create mode 100644 nbd/nbd-internal.h
rename nbd.c => nbd/server.c (68%)
diff --git a/MAINTAINERS b/MAINTAINERS
index d8b0f36..8f44dca 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1116,8 +1116,9 @@ F: net/netmap.c
Network Block Device (NBD)
M: Paolo Bonzini <pbonzini@redhat.com>
S: Odd Fixes
-F: block/nbd.c
-F: nbd.*
+F: block/nbd*
+F: nbd/
+F: include/block/nbd*
F: qemu-nbd.c
T: git git://github.com/bonzini/qemu.git nbd-next
diff --git a/Makefile.objs b/Makefile.objs
index dac2c02..06b95c7 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -8,7 +8,8 @@ util-obj-y += qmp-introspect.o qapi-types.o qapi-visit.o qapi-event.o
# block-obj-y is code used by both qemu system emulation and qemu-img
block-obj-y = async.o thread-pool.o
-block-obj-y += nbd.o block.o blockjob.o
+block-obj-y += nbd/
+block-obj-y += block.o blockjob.o
block-obj-y += main-loop.o iohandler.o qemu-timer.o
block-obj-$(CONFIG_POSIX) += aio-posix.o
block-obj-$(CONFIG_WIN32) += aio-win32.o
diff --git a/nbd/Makefile.objs b/nbd/Makefile.objs
new file mode 100644
index 0000000..eb3dd44
--- /dev/null
+++ b/nbd/Makefile.objs
@@ -0,0 +1 @@
+block-obj-y += server.o client.o common.o
diff --git a/nbd/client.c b/nbd/client.c
new file mode 100644
index 0000000..83df7ba
--- /dev/null
+++ b/nbd/client.c
@@ -0,0 +1,361 @@
+/*
+ * Copyright (C) 2005 Anthony Liguori <anthony@codemonkey.ws>
+ *
+ * Network Block Device Client Side
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; under version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "nbd-internal.h"
+
+static int nbd_errno_to_system_errno(int err)
+{
+ switch (err) {
+ case NBD_SUCCESS:
+ return 0;
+ case NBD_EPERM:
+ return EPERM;
+ case NBD_EIO:
+ return EIO;
+ case NBD_ENOMEM:
+ return ENOMEM;
+ case NBD_ENOSPC:
+ return ENOSPC;
+ case NBD_EINVAL:
+ default:
+ return EINVAL;
+ }
+}
+
+/* Definitions for opaque data types */
+
+static QTAILQ_HEAD(, NBDExport) exports = QTAILQ_HEAD_INITIALIZER(exports);
+
+/* That's all folks */
+
+/* Basic flow for negotiation
+
+ Server Client
+ Negotiate
+
+ or
+
+ Server Client
+ Negotiate #1
+ Option
+ Negotiate #2
+
+ ----
+
+ followed by
+
+ Server Client
+ Request
+ Response
+ Request
+ Response
+ ...
+ ...
+ Request (type == 2)
+
+*/
+
+int nbd_receive_negotiate(int csock, const char *name, uint32_t *flags,
+ off_t *size, Error **errp)
+{
+ char buf[256];
+ uint64_t magic, s;
+ uint16_t tmp;
+ int rc;
+
+ TRACE("Receiving negotiation.");
+
+ rc = -EINVAL;
+
+ if (read_sync(csock, buf, 8) != 8) {
+ error_setg(errp, "Failed to read data");
+ goto fail;
+ }
+
+ buf[8] = '\0';
+ if (strlen(buf) == 0) {
+ error_setg(errp, "Server connection closed unexpectedly");
+ goto fail;
+ }
+
+ TRACE("Magic is %c%c%c%c%c%c%c%c",
+ qemu_isprint(buf[0]) ? buf[0] : '.',
+ qemu_isprint(buf[1]) ? buf[1] : '.',
+ qemu_isprint(buf[2]) ? buf[2] : '.',
+ qemu_isprint(buf[3]) ? buf[3] : '.',
+ qemu_isprint(buf[4]) ? buf[4] : '.',
+ qemu_isprint(buf[5]) ? buf[5] : '.',
+ qemu_isprint(buf[6]) ? buf[6] : '.',
+ qemu_isprint(buf[7]) ? buf[7] : '.');
+
+ if (memcmp(buf, "NBDMAGIC", 8) != 0) {
+ error_setg(errp, "Invalid magic received");
+ goto fail;
+ }
+
+ if (read_sync(csock, &magic, sizeof(magic)) != sizeof(magic)) {
+ error_setg(errp, "Failed to read magic");
+ goto fail;
+ }
+ magic = be64_to_cpu(magic);
+ TRACE("Magic is 0x%" PRIx64, magic);
+
+ if (name) {
+ uint32_t reserved = 0;
+ uint32_t opt;
+ uint32_t namesize;
+
+ TRACE("Checking magic (opts_magic)");
+ if (magic != NBD_OPTS_MAGIC) {
+ if (magic == NBD_CLIENT_MAGIC) {
+ error_setg(errp, "Server does not support export names");
+ } else {
+ error_setg(errp, "Bad magic received");
+ }
+ goto fail;
+ }
+ if (read_sync(csock, &tmp, sizeof(tmp)) != sizeof(tmp)) {
+ error_setg(errp, "Failed to read server flags");
+ goto fail;
+ }
+ *flags = be16_to_cpu(tmp) << 16;
+ /* reserved for future use */
+ if (write_sync(csock, &reserved, sizeof(reserved)) !=
+ sizeof(reserved)) {
+ error_setg(errp, "Failed to read reserved field");
+ goto fail;
+ }
+ /* write the export name */
+ magic = cpu_to_be64(magic);
+ if (write_sync(csock, &magic, sizeof(magic)) != sizeof(magic)) {
+ error_setg(errp, "Failed to send export name magic");
+ goto fail;
+ }
+ opt = cpu_to_be32(NBD_OPT_EXPORT_NAME);
+ if (write_sync(csock, &opt, sizeof(opt)) != sizeof(opt)) {
+ error_setg(errp, "Failed to send export name option number");
+ goto fail;
+ }
+ namesize = cpu_to_be32(strlen(name));
+ if (write_sync(csock, &namesize, sizeof(namesize)) !=
+ sizeof(namesize)) {
+ error_setg(errp, "Failed to send export name length");
+ goto fail;
+ }
+ if (write_sync(csock, (char*)name, strlen(name)) != strlen(name)) {
+ error_setg(errp, "Failed to send export name");
+ goto fail;
+ }
+ } else {
+ TRACE("Checking magic (cli_magic)");
+
+ if (magic != NBD_CLIENT_MAGIC) {
+ if (magic == NBD_OPTS_MAGIC) {
+ error_setg(errp, "Server requires an export name");
+ } else {
+ error_setg(errp, "Bad magic received");
+ }
+ goto fail;
+ }
+ }
+
+ if (read_sync(csock, &s, sizeof(s)) != sizeof(s)) {
+ error_setg(errp, "Failed to read export length");
+ goto fail;
+ }
+ *size = be64_to_cpu(s);
+ TRACE("Size is %" PRIu64, *size);
+
+ if (!name) {
+ if (read_sync(csock, flags, sizeof(*flags)) != sizeof(*flags)) {
+ error_setg(errp, "Failed to read export flags");
+ goto fail;
+ }
+ *flags = be32_to_cpup(flags);
+ } else {
+ if (read_sync(csock, &tmp, sizeof(tmp)) != sizeof(tmp)) {
+ error_setg(errp, "Failed to read export flags");
+ goto fail;
+ }
+ *flags |= be16_to_cpu(tmp);
+ }
+ if (read_sync(csock, &buf, 124) != 124) {
+ error_setg(errp, "Failed to read reserved block");
+ goto fail;
+ }
+ rc = 0;
+
+fail:
+ return rc;
+}
+
+#ifdef __linux__
+int nbd_init(int fd, int csock, uint32_t flags, off_t size)
+{
+ TRACE("Setting NBD socket");
+
+ if (ioctl(fd, NBD_SET_SOCK, csock) < 0) {
+ int serrno = errno;
+ LOG("Failed to set NBD socket");
+ return -serrno;
+ }
+
+ TRACE("Setting block size to %lu", (unsigned long)BDRV_SECTOR_SIZE);
+
+ if (ioctl(fd, NBD_SET_BLKSIZE, (size_t)BDRV_SECTOR_SIZE) < 0) {
+ int serrno = errno;
+ LOG("Failed setting NBD block size");
+ return -serrno;
+ }
+
+ TRACE("Setting size to %zd block(s)", (size_t)(size / BDRV_SECTOR_SIZE));
+
+ if (ioctl(fd, NBD_SET_SIZE_BLOCKS, (size_t)(size / BDRV_SECTOR_SIZE)) < 0) {
+ int serrno = errno;
+ LOG("Failed setting size (in blocks)");
+ return -serrno;
+ }
+
+ if (ioctl(fd, NBD_SET_FLAGS, flags) < 0) {
+ if (errno == ENOTTY) {
+ int read_only = (flags & NBD_FLAG_READ_ONLY) != 0;
+ TRACE("Setting readonly attribute");
+
+ if (ioctl(fd, BLKROSET, (unsigned long) &read_only) < 0) {
+ int serrno = errno;
+ LOG("Failed setting read-only attribute");
+ return -serrno;
+ }
+ } else {
+ int serrno = errno;
+ LOG("Failed setting flags");
+ return -serrno;
+ }
+ }
+
+ TRACE("Negotiation ended");
+
+ return 0;
+}
+
+int nbd_client(int fd)
+{
+ int ret;
+ int serrno;
+
+ TRACE("Doing NBD loop");
+
+ ret = ioctl(fd, NBD_DO_IT);
+ if (ret < 0 && errno == EPIPE) {
+ /* NBD_DO_IT normally returns EPIPE when someone has disconnected
+ * the socket via NBD_DISCONNECT. We do not want to return 1 in
+ * that case.
+ */
+ ret = 0;
+ }
+ serrno = errno;
+
+ TRACE("NBD loop returned %d: %s", ret, strerror(serrno));
+
+ TRACE("Clearing NBD queue");
+ ioctl(fd, NBD_CLEAR_QUE);
+
+ TRACE("Clearing NBD socket");
+ ioctl(fd, NBD_CLEAR_SOCK);
+
+ errno = serrno;
+ return ret;
+}
+#else
+int nbd_init(int fd, int csock, uint32_t flags, off_t size)
+{
+ return -ENOTSUP;
+}
+
+int nbd_client(int fd)
+{
+ return -ENOTSUP;
+}
+#endif
+
+ssize_t nbd_send_request(int csock, struct nbd_request *request)
+{
+ uint8_t buf[NBD_REQUEST_SIZE];
+ ssize_t ret;
+
+ cpu_to_be32w((uint32_t*)buf, NBD_REQUEST_MAGIC);
+ cpu_to_be32w((uint32_t*)(buf + 4), request->type);
+ cpu_to_be64w((uint64_t*)(buf + 8), request->handle);
+ cpu_to_be64w((uint64_t*)(buf + 16), request->from);
+ cpu_to_be32w((uint32_t*)(buf + 24), request->len);
+
+ TRACE("Sending request to client: "
+ "{ .from = %" PRIu64", .len = %u, .handle = %" PRIu64", .type=%i}",
+ request->from, request->len, request->handle, request->type);
+
+ ret = write_sync(csock, buf, sizeof(buf));
+ if (ret < 0) {
+ return ret;
+ }
+
+ if (ret != sizeof(buf)) {
+ LOG("writing to socket failed");
+ return -EINVAL;
+ }
+ return 0;
+}
+
+ssize_t nbd_receive_reply(int csock, struct nbd_reply *reply)
+{
+ uint8_t buf[NBD_REPLY_SIZE];
+ uint32_t magic;
+ ssize_t ret;
+
+ ret = read_sync(csock, buf, sizeof(buf));
+ if (ret < 0) {
+ return ret;
+ }
+
+ if (ret != sizeof(buf)) {
+ LOG("read failed");
+ return -EINVAL;
+ }
+
+ /* Reply
+ [ 0 .. 3] magic (NBD_REPLY_MAGIC)
+ [ 4 .. 7] error (0 == no error)
+ [ 7 .. 15] handle
+ */
+
+ magic = be32_to_cpup((uint32_t*)buf);
+ reply->error = be32_to_cpup((uint32_t*)(buf + 4));
+ reply->handle = be64_to_cpup((uint64_t*)(buf + 8));
+
+ reply->error = nbd_errno_to_system_errno(reply->error);
+
+ TRACE("Got reply: "
+ "{ magic = 0x%x, .error = %d, handle = %" PRIu64" }",
+ magic, reply->error, reply->handle);
+
+ if (magic != NBD_REPLY_MAGIC) {
+ LOG("invalid magic (got 0x%x)", magic);
+ return -EINVAL;
+ }
+ return 0;
+}
+
diff --git a/nbd/common.c b/nbd/common.c
new file mode 100644
index 0000000..7b089b0
--- /dev/null
+++ b/nbd/common.c
@@ -0,0 +1,64 @@
+/*
+ * Copyright (C) 2005 Anthony Liguori <anthony@codemonkey.ws>
+ *
+ * Network Block Device Common Code
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; under version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "nbd-internal.h"
+
+ssize_t nbd_wr_sync(int fd, void *buffer, size_t size, bool do_read)
+{
+ size_t offset = 0;
+ int err;
+
+ if (qemu_in_coroutine()) {
+ if (do_read) {
+ return qemu_co_recv(fd, buffer, size);
+ } else {
+ return qemu_co_send(fd, buffer, size);
+ }
+ }
+
+ while (offset < size) {
+ ssize_t len;
+
+ if (do_read) {
+ len = qemu_recv(fd, buffer + offset, size - offset, 0);
+ } else {
+ len = send(fd, buffer + offset, size - offset, 0);
+ }
+
+ if (len < 0) {
+ err = socket_error();
+
+ /* recoverable error */
+ if (err == EINTR || (offset > 0 && (err == EAGAIN || err == EWOULDBLOCK))) {
+ continue;
+ }
+
+ /* unrecoverable error */
+ return -err;
+ }
+
+ /* eof */
+ if (len == 0) {
+ break;
+ }
+
+ offset += len;
+ }
+
+ return offset;
+}
diff --git a/nbd/nbd-internal.h b/nbd/nbd-internal.h
new file mode 100644
index 0000000..c0a6575
--- /dev/null
+++ b/nbd/nbd-internal.h
@@ -0,0 +1,113 @@
+/*
+ * NBD Internal Declarations
+ *
+ * Copyright (C) 2016 Red Hat, Inc.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#ifndef NBD_INTERNAL_H
+#define NBD_INTERNAL_H
+#include "block/nbd.h"
+#include "sysemu/block-backend.h"
+
+#include "qemu/coroutine.h"
+
+#include <errno.h>
+#include <string.h>
+#ifndef _WIN32
+#include <sys/ioctl.h>
+#endif
+#if defined(__sun__) || defined(__HAIKU__)
+#include <sys/ioccom.h>
+#endif
+#include <ctype.h>
+#include <inttypes.h>
+
+#ifdef __linux__
+#include <linux/fs.h>
+#endif
+
+#include "qemu/sockets.h"
+#include "qemu/queue.h"
+#include "qemu/main-loop.h"
+
+/* #define DEBUG_NBD */
+
+#ifdef DEBUG_NBD
+#define TRACE(msg, ...) do { \
+ LOG(msg, ## __VA_ARGS__); \
+} while(0)
+#else
+#define TRACE(msg, ...) \
+ do { } while (0)
+#endif
+
+#define LOG(msg, ...) do { \
+ fprintf(stderr, "%s:%s():L%d: " msg "\n", \
+ __FILE__, __FUNCTION__, __LINE__, ## __VA_ARGS__); \
+} while(0)
+
+/* This is all part of the "official" NBD API.
+ *
+ * The most up-to-date documentation is available at:
+ * https://github.com/yoe/nbd/blob/master/doc/proto.txt
+ */
+
+#define NBD_REQUEST_SIZE (4 + 4 + 8 + 8 + 4)
+#define NBD_REPLY_SIZE (4 + 4 + 8)
+#define NBD_REQUEST_MAGIC 0x25609513
+#define NBD_REPLY_MAGIC 0x67446698
+#define NBD_OPTS_MAGIC 0x49484156454F5054LL
+#define NBD_CLIENT_MAGIC 0x0000420281861253LL
+#define NBD_REP_MAGIC 0x3e889045565a9LL
+
+#define NBD_SET_SOCK _IO(0xab, 0)
+#define NBD_SET_BLKSIZE _IO(0xab, 1)
+#define NBD_SET_SIZE _IO(0xab, 2)
+#define NBD_DO_IT _IO(0xab, 3)
+#define NBD_CLEAR_SOCK _IO(0xab, 4)
+#define NBD_CLEAR_QUE _IO(0xab, 5)
+#define NBD_PRINT_DEBUG _IO(0xab, 6)
+#define NBD_SET_SIZE_BLOCKS _IO(0xab, 7)
+#define NBD_DISCONNECT _IO(0xab, 8)
+#define NBD_SET_TIMEOUT _IO(0xab, 9)
+#define NBD_SET_FLAGS _IO(0xab, 10)
+
+#define NBD_OPT_EXPORT_NAME (1)
+#define NBD_OPT_ABORT (2)
+#define NBD_OPT_LIST (3)
+
+/* NBD errors are based on errno numbers, so there is a 1:1 mapping,
+ * but only a limited set of errno values is specified in the protocol.
+ * Everything else is squashed to EINVAL.
+ */
+#define NBD_SUCCESS 0
+#define NBD_EPERM 1
+#define NBD_EIO 5
+#define NBD_ENOMEM 12
+#define NBD_EINVAL 22
+#define NBD_ENOSPC 28
+
+static inline ssize_t read_sync(int fd, void *buffer, size_t size)
+{
+ /* Sockets are kept in blocking mode in the negotiation phase. After
+ * that, a non-readable socket simply means that another thread stole
+ * our request/reply. Synchronization is done with recv_coroutine, so
+ * that this is coroutine-safe.
+ */
+ return nbd_wr_sync(fd, buffer, size, true);
+}
+
+static inline ssize_t write_sync(int fd, void *buffer, size_t size)
+{
+ int ret;
+ do {
+ /* For writes, we do expect the socket to be writable. */
+ ret = nbd_wr_sync(fd, buffer, size, false);
+ } while (ret == -EAGAIN);
+ return ret;
+}
+
+#endif
diff --git a/nbd.c b/nbd/server.c
similarity index 68%
rename from nbd.c
rename to nbd/server.c
index f8d0221..ba25ce3 100644
--- a/nbd.c
+++ b/nbd/server.c
@@ -1,7 +1,7 @@
/*
* Copyright (C) 2005 Anthony Liguori <anthony@codemonkey.ws>
*
- * Network Block Device
+ * Network Block Device Server Side
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
@@ -16,86 +16,7 @@
* along with this program; if not, see <http://www.gnu.org/licenses/>.
*/
-#include "block/nbd.h"
-#include "sysemu/block-backend.h"
-
-#include "qemu/coroutine.h"
-
-#include <errno.h>
-#include <string.h>
-#ifndef _WIN32
-#include <sys/ioctl.h>
-#endif
-#if defined(__sun__) || defined(__HAIKU__)
-#include <sys/ioccom.h>
-#endif
-#include <ctype.h>
-#include <inttypes.h>
-
-#ifdef __linux__
-#include <linux/fs.h>
-#endif
-
-#include "qemu/sockets.h"
-#include "qemu/queue.h"
-#include "qemu/main-loop.h"
-
-//#define DEBUG_NBD
-
-#ifdef DEBUG_NBD
-#define TRACE(msg, ...) do { \
- LOG(msg, ## __VA_ARGS__); \
-} while(0)
-#else
-#define TRACE(msg, ...) \
- do { } while (0)
-#endif
-
-#define LOG(msg, ...) do { \
- fprintf(stderr, "%s:%s():L%d: " msg "\n", \
- __FILE__, __FUNCTION__, __LINE__, ## __VA_ARGS__); \
-} while(0)
-
-/* This is all part of the "official" NBD API.
- *
- * The most up-to-date documentation is available at:
- * https://github.com/yoe/nbd/blob/master/doc/proto.txt
- */
-
-#define NBD_REQUEST_SIZE (4 + 4 + 8 + 8 + 4)
-#define NBD_REPLY_SIZE (4 + 4 + 8)
-#define NBD_REQUEST_MAGIC 0x25609513
-#define NBD_REPLY_MAGIC 0x67446698
-#define NBD_OPTS_MAGIC 0x49484156454F5054LL
-#define NBD_CLIENT_MAGIC 0x0000420281861253LL
-#define NBD_REP_MAGIC 0x3e889045565a9LL
-
-#define NBD_SET_SOCK _IO(0xab, 0)
-#define NBD_SET_BLKSIZE _IO(0xab, 1)
-#define NBD_SET_SIZE _IO(0xab, 2)
-#define NBD_DO_IT _IO(0xab, 3)
-#define NBD_CLEAR_SOCK _IO(0xab, 4)
-#define NBD_CLEAR_QUE _IO(0xab, 5)
-#define NBD_PRINT_DEBUG _IO(0xab, 6)
-#define NBD_SET_SIZE_BLOCKS _IO(0xab, 7)
-#define NBD_DISCONNECT _IO(0xab, 8)
-#define NBD_SET_TIMEOUT _IO(0xab, 9)
-#define NBD_SET_FLAGS _IO(0xab, 10)
-
-#define NBD_OPT_EXPORT_NAME (1)
-#define NBD_OPT_ABORT (2)
-#define NBD_OPT_LIST (3)
-
-/* NBD errors are based on errno numbers, so there is a 1:1 mapping,
- * but only a limited set of errno values is specified in the protocol.
- * Everything else is squashed to EINVAL.
- */
-#define NBD_SUCCESS 0
-#define NBD_EPERM 1
-#define NBD_EIO 5
-#define NBD_ENOMEM 12
-#define NBD_EINVAL 22
-#define NBD_ENOSPC 28
+#include "nbd-internal.h"
static int system_errno_to_nbd_errno(int err)
{
@@ -120,25 +41,6 @@ static int system_errno_to_nbd_errno(int err)
}
}
-static int nbd_errno_to_system_errno(int err)
-{
- switch (err) {
- case NBD_SUCCESS:
- return 0;
- case NBD_EPERM:
- return EPERM;
- case NBD_EIO:
- return EIO;
- case NBD_ENOMEM:
- return ENOMEM;
- case NBD_ENOSPC:
- return ENOSPC;
- case NBD_EINVAL:
- default:
- return EINVAL;
- }
-}
-
/* Definitions for opaque data types */
typedef struct NBDRequest NBDRequest;
@@ -191,61 +93,6 @@ static void nbd_set_handlers(NBDClient *client);
static void nbd_unset_handlers(NBDClient *client);
static void nbd_update_can_read(NBDClient *client);
-ssize_t nbd_wr_sync(int fd, void *buffer, size_t size, bool do_read)
-{
- size_t offset = 0;
- int err;
-
- if (qemu_in_coroutine()) {
- if (do_read) {
- return qemu_co_recv(fd, buffer, size);
- } else {
- return qemu_co_send(fd, buffer, size);
- }
- }
-
- while (offset < size) {
- ssize_t len;
-
- if (do_read) {
- len = qemu_recv(fd, buffer + offset, size - offset, 0);
- } else {
- len = send(fd, buffer + offset, size - offset, 0);
- }
-
- if (len < 0) {
- err = socket_error();
-
- /* recoverable error */
- if (err == EINTR || (offset > 0 && (err == EAGAIN || err == EWOULDBLOCK))) {
- continue;
- }
-
- /* unrecoverable error */
- return -err;
- }
-
- /* eof */
- if (len == 0) {
- break;
- }
-
- offset += len;
- }
-
- return offset;
-}
-
-static ssize_t read_sync(int fd, void *buffer, size_t size)
-{
- /* Sockets are kept in blocking mode in the negotiation phase. After
- * that, a non-readable socket simply means that another thread stole
- * our request/reply. Synchronization is done with recv_coroutine, so
- * that this is coroutine-safe.
- */
- return nbd_wr_sync(fd, buffer, size, true);
-}
-
static ssize_t drop_sync(int fd, size_t size)
{
ssize_t ret, dropped = size;
@@ -266,16 +113,6 @@ static ssize_t drop_sync(int fd, size_t size)
return dropped;
}
-static ssize_t write_sync(int fd, void *buffer, size_t size)
-{
- int ret;
- do {
- /* For writes, we do expect the socket to be writable. */
- ret = nbd_wr_sync(fd, buffer, size, false);
- } while (ret == -EAGAIN);
- return ret;
-}
-
/* Basic flow for negotiation
Server Client
@@ -579,188 +416,7 @@ fail:
return rc;
}
-int nbd_receive_negotiate(int csock, const char *name, uint32_t *flags,
- off_t *size, Error **errp)
-{
- char buf[256];
- uint64_t magic, s;
- uint16_t tmp;
- int rc;
-
- TRACE("Receiving negotiation.");
-
- rc = -EINVAL;
-
- if (read_sync(csock, buf, 8) != 8) {
- error_setg(errp, "Failed to read data");
- goto fail;
- }
-
- buf[8] = '\0';
- if (strlen(buf) == 0) {
- error_setg(errp, "Server connection closed unexpectedly");
- goto fail;
- }
-
- TRACE("Magic is %c%c%c%c%c%c%c%c",
- qemu_isprint(buf[0]) ? buf[0] : '.',
- qemu_isprint(buf[1]) ? buf[1] : '.',
- qemu_isprint(buf[2]) ? buf[2] : '.',
- qemu_isprint(buf[3]) ? buf[3] : '.',
- qemu_isprint(buf[4]) ? buf[4] : '.',
- qemu_isprint(buf[5]) ? buf[5] : '.',
- qemu_isprint(buf[6]) ? buf[6] : '.',
- qemu_isprint(buf[7]) ? buf[7] : '.');
-
- if (memcmp(buf, "NBDMAGIC", 8) != 0) {
- error_setg(errp, "Invalid magic received");
- goto fail;
- }
-
- if (read_sync(csock, &magic, sizeof(magic)) != sizeof(magic)) {
- error_setg(errp, "Failed to read magic");
- goto fail;
- }
- magic = be64_to_cpu(magic);
- TRACE("Magic is 0x%" PRIx64, magic);
-
- if (name) {
- uint32_t reserved = 0;
- uint32_t opt;
- uint32_t namesize;
-
- TRACE("Checking magic (opts_magic)");
- if (magic != NBD_OPTS_MAGIC) {
- if (magic == NBD_CLIENT_MAGIC) {
- error_setg(errp, "Server does not support export names");
- } else {
- error_setg(errp, "Bad magic received");
- }
- goto fail;
- }
- if (read_sync(csock, &tmp, sizeof(tmp)) != sizeof(tmp)) {
- error_setg(errp, "Failed to read server flags");
- goto fail;
- }
- *flags = be16_to_cpu(tmp) << 16;
- /* reserved for future use */
- if (write_sync(csock, &reserved, sizeof(reserved)) !=
- sizeof(reserved)) {
- error_setg(errp, "Failed to read reserved field");
- goto fail;
- }
- /* write the export name */
- magic = cpu_to_be64(magic);
- if (write_sync(csock, &magic, sizeof(magic)) != sizeof(magic)) {
- error_setg(errp, "Failed to send export name magic");
- goto fail;
- }
- opt = cpu_to_be32(NBD_OPT_EXPORT_NAME);
- if (write_sync(csock, &opt, sizeof(opt)) != sizeof(opt)) {
- error_setg(errp, "Failed to send export name option number");
- goto fail;
- }
- namesize = cpu_to_be32(strlen(name));
- if (write_sync(csock, &namesize, sizeof(namesize)) !=
- sizeof(namesize)) {
- error_setg(errp, "Failed to send export name length");
- goto fail;
- }
- if (write_sync(csock, (char*)name, strlen(name)) != strlen(name)) {
- error_setg(errp, "Failed to send export name");
- goto fail;
- }
- } else {
- TRACE("Checking magic (cli_magic)");
-
- if (magic != NBD_CLIENT_MAGIC) {
- if (magic == NBD_OPTS_MAGIC) {
- error_setg(errp, "Server requires an export name");
- } else {
- error_setg(errp, "Bad magic received");
- }
- goto fail;
- }
- }
-
- if (read_sync(csock, &s, sizeof(s)) != sizeof(s)) {
- error_setg(errp, "Failed to read export length");
- goto fail;
- }
- *size = be64_to_cpu(s);
- TRACE("Size is %" PRIu64, *size);
-
- if (!name) {
- if (read_sync(csock, flags, sizeof(*flags)) != sizeof(*flags)) {
- error_setg(errp, "Failed to read export flags");
- goto fail;
- }
- *flags = be32_to_cpup(flags);
- } else {
- if (read_sync(csock, &tmp, sizeof(tmp)) != sizeof(tmp)) {
- error_setg(errp, "Failed to read export flags");
- goto fail;
- }
- *flags |= be16_to_cpu(tmp);
- }
- if (read_sync(csock, &buf, 124) != 124) {
- error_setg(errp, "Failed to read reserved block");
- goto fail;
- }
- rc = 0;
-
-fail:
- return rc;
-}
-
#ifdef __linux__
-int nbd_init(int fd, int csock, uint32_t flags, off_t size)
-{
- TRACE("Setting NBD socket");
-
- if (ioctl(fd, NBD_SET_SOCK, csock) < 0) {
- int serrno = errno;
- LOG("Failed to set NBD socket");
- return -serrno;
- }
-
- TRACE("Setting block size to %lu", (unsigned long)BDRV_SECTOR_SIZE);
-
- if (ioctl(fd, NBD_SET_BLKSIZE, (size_t)BDRV_SECTOR_SIZE) < 0) {
- int serrno = errno;
- LOG("Failed setting NBD block size");
- return -serrno;
- }
-
- TRACE("Setting size to %zd block(s)", (size_t)(size / BDRV_SECTOR_SIZE));
-
- if (ioctl(fd, NBD_SET_SIZE_BLOCKS, (size_t)(size / BDRV_SECTOR_SIZE)) < 0) {
- int serrno = errno;
- LOG("Failed setting size (in blocks)");
- return -serrno;
- }
-
- if (ioctl(fd, NBD_SET_FLAGS, flags) < 0) {
- if (errno == ENOTTY) {
- int read_only = (flags & NBD_FLAG_READ_ONLY) != 0;
- TRACE("Setting readonly attribute");
-
- if (ioctl(fd, BLKROSET, (unsigned long) &read_only) < 0) {
- int serrno = errno;
- LOG("Failed setting read-only attribute");
- return -serrno;
- }
- } else {
- int serrno = errno;
- LOG("Failed setting flags");
- return -serrno;
- }
- }
-
- TRACE("Negotiation ended");
-
- return 0;
-}
int nbd_disconnect(int fd)
{
@@ -770,78 +426,14 @@ int nbd_disconnect(int fd)
return 0;
}
-int nbd_client(int fd)
-{
- int ret;
- int serrno;
-
- TRACE("Doing NBD loop");
-
- ret = ioctl(fd, NBD_DO_IT);
- if (ret < 0 && errno == EPIPE) {
- /* NBD_DO_IT normally returns EPIPE when someone has disconnected
- * the socket via NBD_DISCONNECT. We do not want to return 1 in
- * that case.
- */
- ret = 0;
- }
- serrno = errno;
-
- TRACE("NBD loop returned %d: %s", ret, strerror(serrno));
-
- TRACE("Clearing NBD queue");
- ioctl(fd, NBD_CLEAR_QUE);
-
- TRACE("Clearing NBD socket");
- ioctl(fd, NBD_CLEAR_SOCK);
-
- errno = serrno;
- return ret;
-}
#else
-int nbd_init(int fd, int csock, uint32_t flags, off_t size)
-{
- return -ENOTSUP;
-}
int nbd_disconnect(int fd)
{
return -ENOTSUP;
}
-
-int nbd_client(int fd)
-{
- return -ENOTSUP;
-}
#endif
-ssize_t nbd_send_request(int csock, struct nbd_request *request)
-{
- uint8_t buf[NBD_REQUEST_SIZE];
- ssize_t ret;
-
- cpu_to_be32w((uint32_t*)buf, NBD_REQUEST_MAGIC);
- cpu_to_be32w((uint32_t*)(buf + 4), request->type);
- cpu_to_be64w((uint64_t*)(buf + 8), request->handle);
- cpu_to_be64w((uint64_t*)(buf + 16), request->from);
- cpu_to_be32w((uint32_t*)(buf + 24), request->len);
-
- TRACE("Sending request to client: "
- "{ .from = %" PRIu64", .len = %u, .handle = %" PRIu64", .type=%i}",
- request->from, request->len, request->handle, request->type);
-
- ret = write_sync(csock, buf, sizeof(buf));
- if (ret < 0) {
- return ret;
- }
-
- if (ret != sizeof(buf)) {
- LOG("writing to socket failed");
- return -EINVAL;
- }
- return 0;
-}
-
static ssize_t nbd_receive_request(int csock, struct nbd_request *request)
{
uint8_t buf[NBD_REQUEST_SIZE];
@@ -883,45 +475,6 @@ static ssize_t nbd_receive_request(int csock, struct nbd_request *request)
return 0;
}
-ssize_t nbd_receive_reply(int csock, struct nbd_reply *reply)
-{
- uint8_t buf[NBD_REPLY_SIZE];
- uint32_t magic;
- ssize_t ret;
-
- ret = read_sync(csock, buf, sizeof(buf));
- if (ret < 0) {
- return ret;
- }
-
- if (ret != sizeof(buf)) {
- LOG("read failed");
- return -EINVAL;
- }
-
- /* Reply
- [ 0 .. 3] magic (NBD_REPLY_MAGIC)
- [ 4 .. 7] error (0 == no error)
- [ 7 .. 15] handle
- */
-
- magic = be32_to_cpup((uint32_t*)buf);
- reply->error = be32_to_cpup((uint32_t*)(buf + 4));
- reply->handle = be64_to_cpup((uint64_t*)(buf + 8));
-
- reply->error = nbd_errno_to_system_errno(reply->error);
-
- TRACE("Got reply: "
- "{ magic = 0x%x, .error = %d, handle = %" PRIu64" }",
- magic, reply->error, reply->handle);
-
- if (magic != NBD_REPLY_MAGIC) {
- LOG("invalid magic (got 0x%x)", magic);
- return -EINVAL;
- }
- return 0;
-}
-
static ssize_t nbd_send_reply(int csock, struct nbd_reply *reply)
{
uint8_t buf[NBD_REPLY_SIZE];
diff --git a/tests/qemu-iotests/083 b/tests/qemu-iotests/083
index 1b2d3f1..566da99 100755
--- a/tests/qemu-iotests/083
+++ b/tests/qemu-iotests/083
@@ -55,7 +55,7 @@ filter_nbd() {
# callbacks sometimes, making them unreliable.
#
# Filter out the TCP port number since this changes between runs.
- sed -e 's#^.*nbd\.c:.*##g' \
+ sed -e 's#^.*nbd/.*\.c:.*##g' \
-e 's#nbd:127\.0\.0\.1:[^:]*:#nbd:127\.0\.0\.1:PORT:#g' \
-e 's#\(exportname=foo\|PORT\): Failed to .*$#\1#'
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* [Qemu-devel] [PULL 11/15] nbd-server: Coroutine based negotiation
2016-01-15 16:04 [Qemu-devel] [PULL 00/15] NBD, chardev, SCSI patches for 2015-01-15 Paolo Bonzini
` (10 preceding siblings ...)
2016-01-15 16:04 ` [Qemu-devel] [PULL 10/15] nbd: Split nbd.c Paolo Bonzini
@ 2016-01-15 16:04 ` Paolo Bonzini
2016-01-15 16:04 ` [Qemu-devel] [PULL 12/15] nbd-server: do not check request length except for reads and writes Paolo Bonzini
` (4 subsequent siblings)
16 siblings, 0 replies; 30+ messages in thread
From: Paolo Bonzini @ 2016-01-15 16:04 UTC (permalink / raw)
To: qemu-devel; +Cc: Fam Zheng
From: Fam Zheng <famz@redhat.com>
Create a coroutine in nbd_client_new, so that nbd_send_negotiate doesn't
need qemu_set_block().
Handlers need to be set temporarily for csock fd in case the coroutine
yields during I/O.
With this, if the other end disappears in the middle of the negotiation,
we don't block the whole event loop.
To make the code clearer, unify all function names that belong to
negotiate, so they are less likely to be misused. This is important
because we rely on negotiation staying in main loop, as commented in
nbd_negotiate_read/write().
Signed-off-by: Fam Zheng <famz@redhat.com>
Message-Id: <1452760863-25350-4-git-send-email-famz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
nbd/server.c | 150 ++++++++++++++++++++++++++++++++++++++++-------------------
1 file changed, 103 insertions(+), 47 deletions(-)
diff --git a/nbd/server.c b/nbd/server.c
index ba25ce3..8752885 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -93,13 +93,45 @@ static void nbd_set_handlers(NBDClient *client);
static void nbd_unset_handlers(NBDClient *client);
static void nbd_update_can_read(NBDClient *client);
-static ssize_t drop_sync(int fd, size_t size)
+static void nbd_negotiate_continue(void *opaque)
+{
+ qemu_coroutine_enter(opaque, NULL);
+}
+
+static ssize_t nbd_negotiate_read(int fd, void *buffer, size_t size)
+{
+ ssize_t ret;
+
+ assert(qemu_in_coroutine());
+ /* Negotiation are always in main loop. */
+ qemu_set_fd_handler(fd, nbd_negotiate_continue, NULL,
+ qemu_coroutine_self());
+ ret = read_sync(fd, buffer, size);
+ qemu_set_fd_handler(fd, NULL, NULL, NULL);
+ return ret;
+
+}
+
+static ssize_t nbd_negotiate_write(int fd, void *buffer, size_t size)
+{
+ ssize_t ret;
+
+ assert(qemu_in_coroutine());
+ /* Negotiation are always in main loop. */
+ qemu_set_fd_handler(fd, NULL, nbd_negotiate_continue,
+ qemu_coroutine_self());
+ ret = write_sync(fd, buffer, size);
+ qemu_set_fd_handler(fd, NULL, NULL, NULL);
+ return ret;
+}
+
+static ssize_t nbd_negotiate_drop_sync(int fd, size_t size)
{
ssize_t ret, dropped = size;
uint8_t *buffer = g_malloc(MIN(65536, size));
while (size > 0) {
- ret = read_sync(fd, buffer, MIN(65536, size));
+ ret = nbd_negotiate_read(fd, buffer, MIN(65536, size));
if (ret < 0) {
g_free(buffer);
return ret;
@@ -140,96 +172,96 @@ static ssize_t drop_sync(int fd, size_t size)
*/
-static int nbd_send_rep(int csock, uint32_t type, uint32_t opt)
+static int nbd_negotiate_send_rep(int csock, uint32_t type, uint32_t opt)
{
uint64_t magic;
uint32_t len;
magic = cpu_to_be64(NBD_REP_MAGIC);
- if (write_sync(csock, &magic, sizeof(magic)) != sizeof(magic)) {
+ if (nbd_negotiate_write(csock, &magic, sizeof(magic)) != sizeof(magic)) {
LOG("write failed (rep magic)");
return -EINVAL;
}
opt = cpu_to_be32(opt);
- if (write_sync(csock, &opt, sizeof(opt)) != sizeof(opt)) {
+ if (nbd_negotiate_write(csock, &opt, sizeof(opt)) != sizeof(opt)) {
LOG("write failed (rep opt)");
return -EINVAL;
}
type = cpu_to_be32(type);
- if (write_sync(csock, &type, sizeof(type)) != sizeof(type)) {
+ if (nbd_negotiate_write(csock, &type, sizeof(type)) != sizeof(type)) {
LOG("write failed (rep type)");
return -EINVAL;
}
len = cpu_to_be32(0);
- if (write_sync(csock, &len, sizeof(len)) != sizeof(len)) {
+ if (nbd_negotiate_write(csock, &len, sizeof(len)) != sizeof(len)) {
LOG("write failed (rep data length)");
return -EINVAL;
}
return 0;
}
-static int nbd_send_rep_list(int csock, NBDExport *exp)
+static int nbd_negotiate_send_rep_list(int csock, NBDExport *exp)
{
uint64_t magic, name_len;
uint32_t opt, type, len;
name_len = strlen(exp->name);
magic = cpu_to_be64(NBD_REP_MAGIC);
- if (write_sync(csock, &magic, sizeof(magic)) != sizeof(magic)) {
+ if (nbd_negotiate_write(csock, &magic, sizeof(magic)) != sizeof(magic)) {
LOG("write failed (magic)");
return -EINVAL;
}
opt = cpu_to_be32(NBD_OPT_LIST);
- if (write_sync(csock, &opt, sizeof(opt)) != sizeof(opt)) {
+ if (nbd_negotiate_write(csock, &opt, sizeof(opt)) != sizeof(opt)) {
LOG("write failed (opt)");
return -EINVAL;
}
type = cpu_to_be32(NBD_REP_SERVER);
- if (write_sync(csock, &type, sizeof(type)) != sizeof(type)) {
+ if (nbd_negotiate_write(csock, &type, sizeof(type)) != sizeof(type)) {
LOG("write failed (reply type)");
return -EINVAL;
}
len = cpu_to_be32(name_len + sizeof(len));
- if (write_sync(csock, &len, sizeof(len)) != sizeof(len)) {
+ if (nbd_negotiate_write(csock, &len, sizeof(len)) != sizeof(len)) {
LOG("write failed (length)");
return -EINVAL;
}
len = cpu_to_be32(name_len);
- if (write_sync(csock, &len, sizeof(len)) != sizeof(len)) {
+ if (nbd_negotiate_write(csock, &len, sizeof(len)) != sizeof(len)) {
LOG("write failed (length)");
return -EINVAL;
}
- if (write_sync(csock, exp->name, name_len) != name_len) {
+ if (nbd_negotiate_write(csock, exp->name, name_len) != name_len) {
LOG("write failed (buffer)");
return -EINVAL;
}
return 0;
}
-static int nbd_handle_list(NBDClient *client, uint32_t length)
+static int nbd_negotiate_handle_list(NBDClient *client, uint32_t length)
{
int csock;
NBDExport *exp;
csock = client->sock;
if (length) {
- if (drop_sync(csock, length) != length) {
+ if (nbd_negotiate_drop_sync(csock, length) != length) {
return -EIO;
}
- return nbd_send_rep(csock, NBD_REP_ERR_INVALID, NBD_OPT_LIST);
+ return nbd_negotiate_send_rep(csock, NBD_REP_ERR_INVALID, NBD_OPT_LIST);
}
/* For each export, send a NBD_REP_SERVER reply. */
QTAILQ_FOREACH(exp, &exports, next) {
- if (nbd_send_rep_list(csock, exp)) {
+ if (nbd_negotiate_send_rep_list(csock, exp)) {
return -EINVAL;
}
}
/* Finish with a NBD_REP_ACK. */
- return nbd_send_rep(csock, NBD_REP_ACK, NBD_OPT_LIST);
+ return nbd_negotiate_send_rep(csock, NBD_REP_ACK, NBD_OPT_LIST);
}
-static int nbd_handle_export_name(NBDClient *client, uint32_t length)
+static int nbd_negotiate_handle_export_name(NBDClient *client, uint32_t length)
{
int rc = -EINVAL, csock = client->sock;
char name[256];
@@ -242,7 +274,7 @@ static int nbd_handle_export_name(NBDClient *client, uint32_t length)
LOG("Bad length received");
goto fail;
}
- if (read_sync(csock, name, length) != length) {
+ if (nbd_negotiate_read(csock, name, length) != length) {
LOG("read failed");
goto fail;
}
@@ -261,7 +293,7 @@ fail:
return rc;
}
-static int nbd_receive_options(NBDClient *client)
+static int nbd_negotiate_options(NBDClient *client)
{
int csock = client->sock;
uint32_t flags;
@@ -280,7 +312,7 @@ static int nbd_receive_options(NBDClient *client)
... Rest of request
*/
- if (read_sync(csock, &flags, sizeof(flags)) != sizeof(flags)) {
+ if (nbd_negotiate_read(csock, &flags, sizeof(flags)) != sizeof(flags)) {
LOG("read failed");
return -EIO;
}
@@ -296,7 +328,7 @@ static int nbd_receive_options(NBDClient *client)
uint32_t tmp, length;
uint64_t magic;
- if (read_sync(csock, &magic, sizeof(magic)) != sizeof(magic)) {
+ if (nbd_negotiate_read(csock, &magic, sizeof(magic)) != sizeof(magic)) {
LOG("read failed");
return -EINVAL;
}
@@ -306,12 +338,13 @@ static int nbd_receive_options(NBDClient *client)
return -EINVAL;
}
- if (read_sync(csock, &tmp, sizeof(tmp)) != sizeof(tmp)) {
+ if (nbd_negotiate_read(csock, &tmp, sizeof(tmp)) != sizeof(tmp)) {
LOG("read failed");
return -EINVAL;
}
- if (read_sync(csock, &length, sizeof(length)) != sizeof(length)) {
+ if (nbd_negotiate_read(csock, &length,
+ sizeof(length)) != sizeof(length)) {
LOG("read failed");
return -EINVAL;
}
@@ -320,7 +353,7 @@ static int nbd_receive_options(NBDClient *client)
TRACE("Checking option");
switch (be32_to_cpu(tmp)) {
case NBD_OPT_LIST:
- ret = nbd_handle_list(client, length);
+ ret = nbd_negotiate_handle_list(client, length);
if (ret < 0) {
return ret;
}
@@ -330,19 +363,25 @@ static int nbd_receive_options(NBDClient *client)
return -EINVAL;
case NBD_OPT_EXPORT_NAME:
- return nbd_handle_export_name(client, length);
+ return nbd_negotiate_handle_export_name(client, length);
default:
tmp = be32_to_cpu(tmp);
LOG("Unsupported option 0x%x", tmp);
- nbd_send_rep(client->sock, NBD_REP_ERR_UNSUP, tmp);
+ nbd_negotiate_send_rep(client->sock, NBD_REP_ERR_UNSUP, tmp);
return -EINVAL;
}
}
}
-static int nbd_send_negotiate(NBDClient *client)
+typedef struct {
+ NBDClient *client;
+ Coroutine *co;
+} NBDClientNewData;
+
+static coroutine_fn int nbd_negotiate(NBDClientNewData *data)
{
+ NBDClient *client = data->client;
int csock = client->sock;
char buf[8 + 8 + 8 + 128];
int rc;
@@ -368,7 +407,6 @@ static int nbd_send_negotiate(NBDClient *client)
[28 .. 151] reserved (0)
*/
- qemu_set_block(csock);
rc = -EINVAL;
TRACE("Beginning negotiation.");
@@ -385,16 +423,16 @@ static int nbd_send_negotiate(NBDClient *client)
}
if (client->exp) {
- if (write_sync(csock, buf, sizeof(buf)) != sizeof(buf)) {
+ if (nbd_negotiate_write(csock, buf, sizeof(buf)) != sizeof(buf)) {
LOG("write failed");
goto fail;
}
} else {
- if (write_sync(csock, buf, 18) != 18) {
+ if (nbd_negotiate_write(csock, buf, 18) != 18) {
LOG("write failed");
goto fail;
}
- rc = nbd_receive_options(client);
+ rc = nbd_negotiate_options(client);
if (rc != 0) {
LOG("option negotiation failed");
goto fail;
@@ -403,7 +441,8 @@ static int nbd_send_negotiate(NBDClient *client)
assert ((client->exp->nbdflags & ~65535) == 0);
cpu_to_be64w((uint64_t*)(buf + 18), client->exp->size);
cpu_to_be16w((uint16_t*)(buf + 26), client->exp->nbdflags | myflags);
- if (write_sync(csock, buf + 18, sizeof(buf) - 18) != sizeof(buf) - 18) {
+ if (nbd_negotiate_write(csock, buf + 18,
+ sizeof(buf) - 18) != sizeof(buf) - 18) {
LOG("write failed");
goto fail;
}
@@ -412,7 +451,6 @@ static int nbd_send_negotiate(NBDClient *client)
TRACE("Negotiation succeeded.");
rc = 0;
fail:
- qemu_set_nonblock(csock);
return rc;
}
@@ -1028,25 +1066,43 @@ static void nbd_update_can_read(NBDClient *client)
}
}
+static coroutine_fn void nbd_co_client_start(void *opaque)
+{
+ NBDClientNewData *data = opaque;
+ NBDClient *client = data->client;
+ NBDExport *exp = client->exp;
+
+ if (exp) {
+ nbd_export_get(exp);
+ }
+ if (nbd_negotiate(data)) {
+ shutdown(client->sock, 2);
+ client->close(client);
+ goto out;
+ }
+ qemu_co_mutex_init(&client->send_lock);
+ nbd_set_handlers(client);
+
+ if (exp) {
+ QTAILQ_INSERT_TAIL(&exp->clients, client, next);
+ }
+out:
+ g_free(data);
+}
+
void nbd_client_new(NBDExport *exp, int csock, void (*close_fn)(NBDClient *))
{
NBDClient *client;
+ NBDClientNewData *data = g_new(NBDClientNewData, 1);
+
client = g_malloc0(sizeof(NBDClient));
client->refcount = 1;
client->exp = exp;
client->sock = csock;
client->can_read = true;
- if (nbd_send_negotiate(client)) {
- shutdown(client->sock, 2);
- close_fn(client);
- return;
- }
client->close = close_fn;
- qemu_co_mutex_init(&client->send_lock);
- nbd_set_handlers(client);
- if (exp) {
- QTAILQ_INSERT_TAIL(&exp->clients, client, next);
- nbd_export_get(exp);
- }
+ data->client = client;
+ data->co = qemu_coroutine_create(nbd_co_client_start);
+ qemu_coroutine_enter(data->co, data);
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* [Qemu-devel] [PULL 12/15] nbd-server: do not check request length except for reads and writes
2016-01-15 16:04 [Qemu-devel] [PULL 00/15] NBD, chardev, SCSI patches for 2015-01-15 Paolo Bonzini
` (11 preceding siblings ...)
2016-01-15 16:04 ` [Qemu-devel] [PULL 11/15] nbd-server: Coroutine based negotiation Paolo Bonzini
@ 2016-01-15 16:04 ` Paolo Bonzini
2016-01-15 16:04 ` [Qemu-devel] [PULL 13/15] nbd-server: do not exit on failed memory allocation Paolo Bonzini
` (3 subsequent siblings)
16 siblings, 0 replies; 30+ messages in thread
From: Paolo Bonzini @ 2016-01-15 16:04 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block
Only reads and writes need to allocate memory correspondent to the
request length. Other requests can be sent to the storage without
allocating any memory, and thus any request length is acceptable.
Reported-by: Sitsofe Wheeler <sitsofe@yahoo.com>
Cc: qemu-block@nongnu.org
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
nbd/server.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/nbd/server.c b/nbd/server.c
index 8752885..c41af0d 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -818,13 +818,6 @@ static ssize_t nbd_co_receive_request(NBDRequest *req, struct nbd_request *reque
goto out;
}
- if (request->len > NBD_MAX_BUFFER_SIZE) {
- LOG("len (%u) is larger than max len (%u)",
- request->len, NBD_MAX_BUFFER_SIZE);
- rc = -EINVAL;
- goto out;
- }
-
if ((request->from + request->len) < request->from) {
LOG("integer overflow detected! "
"you're probably being attacked");
@@ -836,6 +829,13 @@ static ssize_t nbd_co_receive_request(NBDRequest *req, struct nbd_request *reque
command = request->type & NBD_CMD_MASK_COMMAND;
if (command == NBD_CMD_READ || command == NBD_CMD_WRITE) {
+ if (request->len > NBD_MAX_BUFFER_SIZE) {
+ LOG("len (%u) is larger than max len (%u)",
+ request->len, NBD_MAX_BUFFER_SIZE);
+ rc = -EINVAL;
+ goto out;
+ }
+
req->data = blk_blockalign(client->exp->blk, request->len);
}
if (command == NBD_CMD_WRITE) {
--
1.8.3.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* [Qemu-devel] [PULL 13/15] nbd-server: do not exit on failed memory allocation
2016-01-15 16:04 [Qemu-devel] [PULL 00/15] NBD, chardev, SCSI patches for 2015-01-15 Paolo Bonzini
` (12 preceding siblings ...)
2016-01-15 16:04 ` [Qemu-devel] [PULL 12/15] nbd-server: do not check request length except for reads and writes Paolo Bonzini
@ 2016-01-15 16:04 ` Paolo Bonzini
2016-01-15 16:04 ` [Qemu-devel] [PULL 14/15] qemu-char: add logfile facility to all chardev backends Paolo Bonzini
` (2 subsequent siblings)
16 siblings, 0 replies; 30+ messages in thread
From: Paolo Bonzini @ 2016-01-15 16:04 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block
The amount of memory allocated in nbd_co_receive_request is driven by the
NBD client (possibly a virtual machine). Parallel I/O can cause the
server to allocate a large amount of memory; check for failures and
return ENOMEM in that case.
Cc: qemu-block@nongnu.org
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
block/block-backend.c | 5 +++++
include/sysemu/block-backend.h | 1 +
nbd/server.c | 6 +++++-
3 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/block/block-backend.c b/block/block-backend.c
index f41d326..e813759 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1033,6 +1033,11 @@ void blk_set_guest_block_size(BlockBackend *blk, int align)
blk->guest_block_size = align;
}
+void *blk_try_blockalign(BlockBackend *blk, size_t size)
+{
+ return qemu_try_blockalign(blk ? blk->bs : NULL, size);
+}
+
void *blk_blockalign(BlockBackend *blk, size_t size)
{
return qemu_blockalign(blk ? blk->bs : NULL, size);
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index dc24476..1568554 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -148,6 +148,7 @@ int blk_get_flags(BlockBackend *blk);
int blk_get_max_transfer_length(BlockBackend *blk);
int blk_get_max_iov(BlockBackend *blk);
void blk_set_guest_block_size(BlockBackend *blk, int align);
+void *blk_try_blockalign(BlockBackend *blk, size_t size);
void *blk_blockalign(BlockBackend *blk, size_t size);
bool blk_op_is_blocked(BlockBackend *blk, BlockOpType op, Error **errp);
void blk_op_unblock(BlockBackend *blk, BlockOpType op, Error *reason);
diff --git a/nbd/server.c b/nbd/server.c
index c41af0d..eead339 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -836,7 +836,11 @@ static ssize_t nbd_co_receive_request(NBDRequest *req, struct nbd_request *reque
goto out;
}
- req->data = blk_blockalign(client->exp->blk, request->len);
+ req->data = blk_try_blockalign(client->exp->blk, request->len);
+ if (req->data == NULL) {
+ rc = -ENOMEM;
+ goto out;
+ }
}
if (command == NBD_CMD_WRITE) {
TRACE("Reading %u byte(s)", request->len);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* [Qemu-devel] [PULL 14/15] qemu-char: add logfile facility to all chardev backends
2016-01-15 16:04 [Qemu-devel] [PULL 00/15] NBD, chardev, SCSI patches for 2015-01-15 Paolo Bonzini
` (13 preceding siblings ...)
2016-01-15 16:04 ` [Qemu-devel] [PULL 13/15] nbd-server: do not exit on failed memory allocation Paolo Bonzini
@ 2016-01-15 16:04 ` Paolo Bonzini
2016-01-21 6:16 ` Hervé Poussineau
2016-01-15 16:04 ` [Qemu-devel] [PULL 15/15] qemu-char: do not leak QemuMutex when freeing a character device Paolo Bonzini
2016-01-15 17:42 ` [Qemu-devel] [PULL 00/15] NBD, chardev, SCSI patches for 2015-01-15 Peter Maydell
16 siblings, 1 reply; 30+ messages in thread
From: Paolo Bonzini @ 2016-01-15 16:04 UTC (permalink / raw)
To: qemu-devel
From: "Daniel P. Berrange" <berrange@redhat.com>
Typically a UNIX guest OS will log boot messages to a serial
port in addition to any graphical console. An admin user
may also wish to use the serial port for an interactive
console. A virtualization management system may wish to
collect system boot messages by logging the serial port,
but also wish to allow admins interactive access.
Currently providing such a feature forces the mgmt app
to either provide 2 separate serial ports, one for
logging boot messages and one for interactive console
login, or to proxy all output via a separate service
that can multiplex the two needs onto one serial port.
While both are valid approaches, they each have their
own downsides. The former causes confusion and extra
setup work for VM admins creating disk images. The latter
places an extra burden to re-implement much of the QEMU
chardev backends logic in libvirt or even higher level
mgmt apps and adds extra hops in the data transfer path.
A simpler approach that is satisfactory for many use
cases is to allow the QEMU chardev backends to have a
"logfile" property associated with them.
$QEMU -chardev socket,host=localhost,port=9000,\
server=on,nowait,id-charserial0,\
logfile=/var/log/libvirt/qemu/test-serial0.log
-device isa-serial,chardev=charserial0,id=serial0
This patch introduces a 'ChardevCommon' struct which
is setup as a base for all the ChardevBackend types.
Ideally this would be registered directly as a base
against ChardevBackend, rather than each type, but
the QAPI generator doesn't allow that since the
ChardevBackend is a non-discriminated union. The
ChardevCommon struct provides the optional 'logfile'
parameter, as well as 'logappend' which controls
whether QEMU truncates or appends (default truncate).
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-Id: <1452516281-27519-1-git-send-email-berrange@redhat.com>
[Call qemu_chr_parse_common if cd->parse is NULL. - Paolo]
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
backends/baum.c | 7 +-
backends/msmouse.c | 6 +-
gdbstub.c | 3 +-
include/sysemu/char.h | 9 +-
qapi-schema.json | 49 +++++++---
qemu-char.c | 248 ++++++++++++++++++++++++++++++++++++++++++--------
qemu-options.hx | 48 ++++++----
spice-qemu-char.c | 20 +++-
ui/console.c | 6 +-
9 files changed, 313 insertions(+), 83 deletions(-)
diff --git a/backends/baum.c b/backends/baum.c
index 723c658..ba32b61 100644
--- a/backends/baum.c
+++ b/backends/baum.c
@@ -566,6 +566,7 @@ static CharDriverState *chr_baum_init(const char *id,
ChardevReturn *ret,
Error **errp)
{
+ ChardevCommon *common = qapi_ChardevDummy_base(backend->u.braille);
BaumDriverState *baum;
CharDriverState *chr;
brlapi_handle_t *handle;
@@ -576,8 +577,12 @@ static CharDriverState *chr_baum_init(const char *id,
#endif
int tty;
+ chr = qemu_chr_alloc(common, errp);
+ if (!chr) {
+ return NULL;
+ }
baum = g_malloc0(sizeof(BaumDriverState));
- baum->chr = chr = qemu_chr_alloc();
+ baum->chr = chr;
chr->opaque = baum;
chr->chr_write = baum_write;
diff --git a/backends/msmouse.c b/backends/msmouse.c
index 0126fa0..476dab5 100644
--- a/backends/msmouse.c
+++ b/backends/msmouse.c
@@ -68,9 +68,13 @@ static CharDriverState *qemu_chr_open_msmouse(const char *id,
ChardevReturn *ret,
Error **errp)
{
+ ChardevCommon *common = qapi_ChardevDummy_base(backend->u.msmouse);
CharDriverState *chr;
- chr = qemu_chr_alloc();
+ chr = qemu_chr_alloc(common, errp);
+ if (!chr) {
+ return NULL;
+ }
chr->chr_write = msmouse_chr_write;
chr->chr_close = msmouse_chr_close;
chr->explicit_be_open = true;
diff --git a/gdbstub.c b/gdbstub.c
index 9c29aa0..1a84c1a 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1732,6 +1732,7 @@ int gdbserver_start(const char *device)
char gdbstub_device_name[128];
CharDriverState *chr = NULL;
CharDriverState *mon_chr;
+ ChardevCommon common = { 0 };
if (!device)
return -1;
@@ -1768,7 +1769,7 @@ int gdbserver_start(const char *device)
qemu_add_vm_change_state_handler(gdb_vm_state_change, NULL);
/* Initialize a monitor terminal for gdb */
- mon_chr = qemu_chr_alloc();
+ mon_chr = qemu_chr_alloc(&common, &error_abort);
mon_chr->chr_write = gdb_monitor_write;
monitor_init(mon_chr, 0);
} else {
diff --git a/include/sysemu/char.h b/include/sysemu/char.h
index aff193f..598dd2b 100644
--- a/include/sysemu/char.h
+++ b/include/sysemu/char.h
@@ -77,6 +77,7 @@ struct CharDriverState {
void *opaque;
char *label;
char *filename;
+ int logfd;
int be_open;
int fe_open;
int explicit_fe_open;
@@ -89,13 +90,15 @@ struct CharDriverState {
};
/**
- * @qemu_chr_alloc:
+ * qemu_chr_alloc:
+ * @backend: the common backend config
+ * @errp: pointer to a NULL-initialized error object
*
* Allocate and initialize a new CharDriverState.
*
- * Returns: a newly allocated CharDriverState.
+ * Returns: a newly allocated CharDriverState, or NULL on error.
*/
-CharDriverState *qemu_chr_alloc(void);
+CharDriverState *qemu_chr_alloc(ChardevCommon *backend, Error **errp);
/**
* @qemu_chr_new_from_opts:
diff --git a/qapi-schema.json b/qapi-schema.json
index 2e31733..dabb5ce 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3093,6 +3093,21 @@
##
{ 'command': 'screendump', 'data': {'filename': 'str'} }
+
+##
+# @ChardevCommon:
+#
+# Configuration shared across all chardev backends
+#
+# @logfile: #optional The name of a logfile to save output
+# @logappend: #optional true to append instead of truncate
+# (default to false to truncate)
+#
+# Since: 2.6
+##
+{ 'struct': 'ChardevCommon', 'data': { '*logfile': 'str',
+ '*logappend': 'bool' } }
+
##
# @ChardevFile:
#
@@ -3107,7 +3122,8 @@
##
{ 'struct': 'ChardevFile', 'data': { '*in' : 'str',
'out' : 'str',
- '*append': 'bool' } }
+ '*append': 'bool' },
+ 'base': 'ChardevCommon' }
##
# @ChardevHostdev:
@@ -3120,7 +3136,8 @@
#
# Since: 1.4
##
-{ 'struct': 'ChardevHostdev', 'data': { 'device' : 'str' } }
+{ 'struct': 'ChardevHostdev', 'data': { 'device' : 'str' },
+ 'base': 'ChardevCommon' }
##
# @ChardevSocket:
@@ -3147,7 +3164,8 @@
'*wait' : 'bool',
'*nodelay' : 'bool',
'*telnet' : 'bool',
- '*reconnect' : 'int' } }
+ '*reconnect' : 'int' },
+ 'base': 'ChardevCommon' }
##
# @ChardevUdp:
@@ -3160,7 +3178,8 @@
# Since: 1.5
##
{ 'struct': 'ChardevUdp', 'data': { 'remote' : 'SocketAddress',
- '*local' : 'SocketAddress' } }
+ '*local' : 'SocketAddress' },
+ 'base': 'ChardevCommon' }
##
# @ChardevMux:
@@ -3171,7 +3190,8 @@
#
# Since: 1.5
##
-{ 'struct': 'ChardevMux', 'data': { 'chardev' : 'str' } }
+{ 'struct': 'ChardevMux', 'data': { 'chardev' : 'str' },
+ 'base': 'ChardevCommon' }
##
# @ChardevStdio:
@@ -3184,7 +3204,9 @@
#
# Since: 1.5
##
-{ 'struct': 'ChardevStdio', 'data': { '*signal' : 'bool' } }
+{ 'struct': 'ChardevStdio', 'data': { '*signal' : 'bool' },
+ 'base': 'ChardevCommon' }
+
##
# @ChardevSpiceChannel:
@@ -3195,7 +3217,8 @@
#
# Since: 1.5
##
-{ 'struct': 'ChardevSpiceChannel', 'data': { 'type' : 'str' } }
+{ 'struct': 'ChardevSpiceChannel', 'data': { 'type' : 'str' },
+ 'base': 'ChardevCommon' }
##
# @ChardevSpicePort:
@@ -3206,7 +3229,8 @@
#
# Since: 1.5
##
-{ 'struct': 'ChardevSpicePort', 'data': { 'fqdn' : 'str' } }
+{ 'struct': 'ChardevSpicePort', 'data': { 'fqdn' : 'str' },
+ 'base': 'ChardevCommon' }
##
# @ChardevVC:
@@ -3223,7 +3247,8 @@
{ 'struct': 'ChardevVC', 'data': { '*width' : 'int',
'*height' : 'int',
'*cols' : 'int',
- '*rows' : 'int' } }
+ '*rows' : 'int' },
+ 'base': 'ChardevCommon' }
##
# @ChardevRingbuf:
@@ -3234,7 +3259,8 @@
#
# Since: 1.5
##
-{ 'struct': 'ChardevRingbuf', 'data': { '*size' : 'int' } }
+{ 'struct': 'ChardevRingbuf', 'data': { '*size' : 'int' },
+ 'base': 'ChardevCommon' }
##
# @ChardevBackend:
@@ -3243,7 +3269,8 @@
#
# Since: 1.4 (testdev since 2.2)
##
-{ 'struct': 'ChardevDummy', 'data': { } }
+{ 'struct': 'ChardevDummy', 'data': { },
+ 'base': 'ChardevCommon' }
{ 'union': 'ChardevBackend', 'data': { 'file' : 'ChardevFile',
'serial' : 'ChardevHostdev',
diff --git a/qemu-char.c b/qemu-char.c
index d7be185..11caa56 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -159,10 +159,33 @@ static int sockaddr_to_str(char *dest, int max_len,
static QTAILQ_HEAD(CharDriverStateHead, CharDriverState) chardevs =
QTAILQ_HEAD_INITIALIZER(chardevs);
-CharDriverState *qemu_chr_alloc(void)
+static void qemu_chr_free_common(CharDriverState *chr);
+
+CharDriverState *qemu_chr_alloc(ChardevCommon *backend, Error **errp)
{
CharDriverState *chr = g_malloc0(sizeof(CharDriverState));
qemu_mutex_init(&chr->chr_write_lock);
+
+ if (backend->has_logfile) {
+ int flags = O_WRONLY | O_CREAT;
+ if (backend->has_logappend &&
+ backend->logappend) {
+ flags |= O_APPEND;
+ } else {
+ flags |= O_TRUNC;
+ }
+ chr->logfd = qemu_open(backend->logfile, flags, 0666);
+ if (chr->logfd < 0) {
+ error_setg_errno(errp, errno,
+ "Unable to open logfile %s",
+ backend->logfile);
+ g_free(chr);
+ return NULL;
+ }
+ } else {
+ chr->logfd = -1;
+ }
+
return chr;
}
@@ -188,12 +211,45 @@ void qemu_chr_be_generic_open(CharDriverState *s)
qemu_chr_be_event(s, CHR_EVENT_OPENED);
}
+
+/* Not reporting errors from writing to logfile, as logs are
+ * defined to be "best effort" only */
+static void qemu_chr_fe_write_log(CharDriverState *s,
+ const uint8_t *buf, size_t len)
+{
+ size_t done = 0;
+ ssize_t ret;
+
+ if (s->logfd < 0) {
+ return;
+ }
+
+ while (done < len) {
+ do {
+ ret = write(s->logfd, buf + done, len - done);
+ if (ret == -1 && errno == EAGAIN) {
+ g_usleep(100);
+ }
+ } while (ret == -1 && errno == EAGAIN);
+
+ if (ret <= 0) {
+ return;
+ }
+ done += ret;
+ }
+}
+
int qemu_chr_fe_write(CharDriverState *s, const uint8_t *buf, int len)
{
int ret;
qemu_mutex_lock(&s->chr_write_lock);
ret = s->chr_write(s, buf, len);
+
+ if (ret > 0) {
+ qemu_chr_fe_write_log(s, buf, ret);
+ }
+
qemu_mutex_unlock(&s->chr_write_lock);
return ret;
}
@@ -218,6 +274,10 @@ int qemu_chr_fe_write_all(CharDriverState *s, const uint8_t *buf, int len)
offset += res;
}
+ if (offset > 0) {
+ qemu_chr_fe_write_log(s, buf, offset);
+ }
+
qemu_mutex_unlock(&s->chr_write_lock);
if (res < 0) {
@@ -365,8 +425,12 @@ static CharDriverState *qemu_chr_open_null(const char *id,
Error **errp)
{
CharDriverState *chr;
+ ChardevCommon *common = qapi_ChardevDummy_base(backend->u.null);
- chr = qemu_chr_alloc();
+ chr = qemu_chr_alloc(common, errp);
+ if (!chr) {
+ return NULL;
+ }
chr->chr_write = null_chr_write;
chr->explicit_be_open = true;
return chr;
@@ -665,6 +729,7 @@ static CharDriverState *qemu_chr_open_mux(const char *id,
ChardevMux *mux = backend->u.mux;
CharDriverState *chr, *drv;
MuxDriver *d;
+ ChardevCommon *common = qapi_ChardevMux_base(backend->u.mux);
drv = qemu_chr_find(mux->chardev);
if (drv == NULL) {
@@ -672,7 +737,10 @@ static CharDriverState *qemu_chr_open_mux(const char *id,
return NULL;
}
- chr = qemu_chr_alloc();
+ chr = qemu_chr_alloc(common, errp);
+ if (!chr) {
+ return NULL;
+ }
d = g_new0(MuxDriver, 1);
chr->opaque = d;
@@ -975,12 +1043,16 @@ static void fd_chr_close(struct CharDriverState *chr)
}
/* open a character device to a unix fd */
-static CharDriverState *qemu_chr_open_fd(int fd_in, int fd_out)
+static CharDriverState *qemu_chr_open_fd(int fd_in, int fd_out,
+ ChardevCommon *backend, Error **errp)
{
CharDriverState *chr;
FDCharDriver *s;
- chr = qemu_chr_alloc();
+ chr = qemu_chr_alloc(backend, errp);
+ if (!chr) {
+ return NULL;
+ }
s = g_new0(FDCharDriver, 1);
s->fd_in = io_channel_from_fd(fd_in);
s->fd_out = io_channel_from_fd(fd_out);
@@ -1005,6 +1077,7 @@ static CharDriverState *qemu_chr_open_pipe(const char *id,
char filename_in[CHR_MAX_FILENAME_SIZE];
char filename_out[CHR_MAX_FILENAME_SIZE];
const char *filename = opts->device;
+ ChardevCommon *common = qapi_ChardevHostdev_base(backend->u.pipe);
snprintf(filename_in, CHR_MAX_FILENAME_SIZE, "%s.in", filename);
snprintf(filename_out, CHR_MAX_FILENAME_SIZE, "%s.out", filename);
@@ -1021,7 +1094,7 @@ static CharDriverState *qemu_chr_open_pipe(const char *id,
return NULL;
}
}
- return qemu_chr_open_fd(fd_in, fd_out);
+ return qemu_chr_open_fd(fd_in, fd_out, common, errp);
}
/* init terminal so that we can grab keys */
@@ -1081,6 +1154,7 @@ static CharDriverState *qemu_chr_open_stdio(const char *id,
ChardevStdio *opts = backend->u.stdio;
CharDriverState *chr;
struct sigaction act;
+ ChardevCommon *common = qapi_ChardevStdio_base(backend->u.stdio);
if (is_daemonized()) {
error_setg(errp, "cannot use stdio with -daemonize");
@@ -1102,7 +1176,7 @@ static CharDriverState *qemu_chr_open_stdio(const char *id,
act.sa_handler = term_stdio_handler;
sigaction(SIGCONT, &act, NULL);
- chr = qemu_chr_open_fd(0, 1);
+ chr = qemu_chr_open_fd(0, 1, common, errp);
chr->chr_close = qemu_chr_close_stdio;
chr->chr_set_echo = qemu_chr_set_echo_stdio;
if (opts->has_signal) {
@@ -1324,6 +1398,7 @@ static CharDriverState *qemu_chr_open_pty(const char *id,
PtyCharDriver *s;
int master_fd, slave_fd;
char pty_name[PATH_MAX];
+ ChardevCommon *common = qapi_ChardevDummy_base(backend->u.pty);
master_fd = qemu_openpty_raw(&slave_fd, pty_name);
if (master_fd < 0) {
@@ -1334,7 +1409,11 @@ static CharDriverState *qemu_chr_open_pty(const char *id,
close(slave_fd);
qemu_set_nonblock(master_fd);
- chr = qemu_chr_alloc();
+ chr = qemu_chr_alloc(common, errp);
+ if (!chr) {
+ close(master_fd);
+ return NULL;
+ }
chr->filename = g_strdup_printf("pty:%s", pty_name);
ret->pty = g_strdup(pty_name);
@@ -1557,12 +1636,14 @@ static void qemu_chr_close_tty(CharDriverState *chr)
}
}
-static CharDriverState *qemu_chr_open_tty_fd(int fd)
+static CharDriverState *qemu_chr_open_tty_fd(int fd,
+ ChardevCommon *backend,
+ Error **errp)
{
CharDriverState *chr;
tty_serial_init(fd, 115200, 'N', 8, 1);
- chr = qemu_chr_open_fd(fd, fd);
+ chr = qemu_chr_open_fd(fd, fd, backend, errp);
chr->chr_ioctl = tty_serial_ioctl;
chr->chr_close = qemu_chr_close_tty;
return chr;
@@ -1682,7 +1763,9 @@ static void pp_close(CharDriverState *chr)
qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
}
-static CharDriverState *qemu_chr_open_pp_fd(int fd, Error **errp)
+static CharDriverState *qemu_chr_open_pp_fd(int fd,
+ ChardevCommon *backend,
+ Error **errp)
{
CharDriverState *chr;
ParallelCharDriver *drv;
@@ -1697,7 +1780,10 @@ static CharDriverState *qemu_chr_open_pp_fd(int fd, Error **errp)
drv->fd = fd;
drv->mode = IEEE1284_MODE_COMPAT;
- chr = qemu_chr_alloc();
+ chr = qemu_chr_alloc(backend, errp);
+ if (!chr) {
+ return NULL;
+ }
chr->chr_write = null_chr_write;
chr->chr_ioctl = pp_ioctl;
chr->chr_close = pp_close;
@@ -1748,11 +1834,16 @@ static int pp_ioctl(CharDriverState *chr, int cmd, void *arg)
return 0;
}
-static CharDriverState *qemu_chr_open_pp_fd(int fd, Error **errp)
+static CharDriverState *qemu_chr_open_pp_fd(int fd,
+ ChardevBackend *backend,
+ Error **errp)
{
CharDriverState *chr;
- chr = qemu_chr_alloc();
+ chr = qemu_chr_alloc(common, errp);
+ if (!chr) {
+ return NULL;
+ }
chr->opaque = (void *)(intptr_t)fd;
chr->chr_write = null_chr_write;
chr->chr_ioctl = pp_ioctl;
@@ -1978,12 +2069,16 @@ static int win_chr_poll(void *opaque)
}
static CharDriverState *qemu_chr_open_win_path(const char *filename,
+ ChardevCommon *backend,
Error **errp)
{
CharDriverState *chr;
WinCharState *s;
- chr = qemu_chr_alloc();
+ chr = qemu_chr_alloc(backend, errp);
+ if (!chr) {
+ return NULL;
+ }
s = g_new0(WinCharState, 1);
chr->opaque = s;
chr->chr_write = win_chr_write;
@@ -1991,7 +2086,7 @@ static CharDriverState *qemu_chr_open_win_path(const char *filename,
if (win_chr_init(chr, filename, errp) < 0) {
g_free(s);
- g_free(chr);
+ qemu_chr_free_common(chr);
return NULL;
}
return chr;
@@ -2086,8 +2181,12 @@ static CharDriverState *qemu_chr_open_pipe(const char *id,
const char *filename = opts->device;
CharDriverState *chr;
WinCharState *s;
+ ChardevCommon *common = qapi_ChardevHostdev_base(backend->u.pipe);
- chr = qemu_chr_alloc();
+ chr = qemu_chr_alloc(common, errp);
+ if (!chr) {
+ return NULL;
+ }
s = g_new0(WinCharState, 1);
chr->opaque = s;
chr->chr_write = win_chr_write;
@@ -2095,18 +2194,23 @@ static CharDriverState *qemu_chr_open_pipe(const char *id,
if (win_chr_pipe_init(chr, filename, errp) < 0) {
g_free(s);
- g_free(chr);
+ qemu_chr_free_common(chr);
return NULL;
}
return chr;
}
-static CharDriverState *qemu_chr_open_win_file(HANDLE fd_out)
+static CharDriverState *qemu_chr_open_win_file(HANDLE fd_out,
+ ChardevCommon *backend,
+ Error **errp)
{
CharDriverState *chr;
WinCharState *s;
- chr = qemu_chr_alloc();
+ chr = qemu_chr_alloc(backend, errp);
+ if (!chr) {
+ return NULL;
+ }
s = g_new0(WinCharState, 1);
s->hcom = fd_out;
chr->opaque = s;
@@ -2119,7 +2223,9 @@ static CharDriverState *qemu_chr_open_win_con(const char *id,
ChardevReturn *ret,
Error **errp)
{
- return qemu_chr_open_win_file(GetStdHandle(STD_OUTPUT_HANDLE));
+ ChardevCommon *common = qapi_ChardevDummy_base(backend->u.console);
+ return qemu_chr_open_win_file(GetStdHandle(STD_OUTPUT_HANDLE),
+ common, errp);
}
static int win_stdio_write(CharDriverState *chr, const uint8_t *buf, int len)
@@ -2267,8 +2373,12 @@ static CharDriverState *qemu_chr_open_stdio(const char *id,
WinStdioCharState *stdio;
DWORD dwMode;
int is_console = 0;
+ ChardevCommon *common = qapi_ChardevStdio_base(backend->u.stdio);
- chr = qemu_chr_alloc();
+ chr = qemu_chr_alloc(common, errp);
+ if (!chr) {
+ return NULL;
+ }
stdio = g_new0(WinStdioCharState, 1);
stdio->hStdIn = GetStdHandle(STD_INPUT_HANDLE);
@@ -2440,12 +2550,17 @@ static void udp_chr_close(CharDriverState *chr)
qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
}
-static CharDriverState *qemu_chr_open_udp_fd(int fd)
+static CharDriverState *qemu_chr_open_udp_fd(int fd,
+ ChardevCommon *backend,
+ Error **errp)
{
CharDriverState *chr = NULL;
NetCharDriver *s = NULL;
- chr = qemu_chr_alloc();
+ chr = qemu_chr_alloc(backend, errp);
+ if (!chr) {
+ return NULL;
+ }
s = g_new0(NetCharDriver, 1);
s->fd = fd;
@@ -2856,7 +2971,7 @@ static int tcp_chr_sync_read(CharDriverState *chr, const uint8_t *buf, int len)
#ifndef _WIN32
CharDriverState *qemu_chr_open_eventfd(int eventfd)
{
- CharDriverState *chr = qemu_chr_open_fd(eventfd, eventfd);
+ CharDriverState *chr = qemu_chr_open_fd(eventfd, eventfd, NULL, NULL);
if (chr) {
chr->avail_connections = 1;
@@ -3138,10 +3253,14 @@ static CharDriverState *qemu_chr_open_ringbuf(const char *id,
Error **errp)
{
ChardevRingbuf *opts = backend->u.ringbuf;
+ ChardevCommon *common = qapi_ChardevRingbuf_base(backend->u.ringbuf);
CharDriverState *chr;
RingBufCharDriver *d;
- chr = qemu_chr_alloc();
+ chr = qemu_chr_alloc(common, errp);
+ if (!chr) {
+ return NULL;
+ }
d = g_malloc(sizeof(*d));
d->size = opts->has_size ? opts->size : 65536;
@@ -3164,7 +3283,7 @@ static CharDriverState *qemu_chr_open_ringbuf(const char *id,
fail:
g_free(d);
- g_free(chr);
+ qemu_chr_free_common(chr);
return NULL;
}
@@ -3408,6 +3527,18 @@ fail:
return NULL;
}
+static void qemu_chr_parse_common(QemuOpts *opts, ChardevCommon *backend)
+{
+ const char *logfile = qemu_opt_get(opts, "logfile");
+
+ backend->has_logfile = logfile != NULL;
+ backend->logfile = logfile ? g_strdup(logfile) : NULL;
+
+ backend->has_logappend = true;
+ backend->logappend = qemu_opt_get_bool(opts, "logappend", false);
+}
+
+
static void qemu_chr_parse_file_out(QemuOpts *opts, ChardevBackend *backend,
Error **errp)
{
@@ -3418,6 +3549,7 @@ static void qemu_chr_parse_file_out(QemuOpts *opts, ChardevBackend *backend,
return;
}
backend->u.file = g_new0(ChardevFile, 1);
+ qemu_chr_parse_common(opts, qapi_ChardevFile_base(backend->u.file));
backend->u.file->out = g_strdup(path);
backend->u.file->has_append = true;
@@ -3428,6 +3560,7 @@ static void qemu_chr_parse_stdio(QemuOpts *opts, ChardevBackend *backend,
Error **errp)
{
backend->u.stdio = g_new0(ChardevStdio, 1);
+ qemu_chr_parse_common(opts, qapi_ChardevStdio_base(backend->u.stdio));
backend->u.stdio->has_signal = true;
backend->u.stdio->signal = qemu_opt_get_bool(opts, "signal", true);
}
@@ -3443,6 +3576,7 @@ static void qemu_chr_parse_serial(QemuOpts *opts, ChardevBackend *backend,
return;
}
backend->u.serial = g_new0(ChardevHostdev, 1);
+ qemu_chr_parse_common(opts, qapi_ChardevHostdev_base(backend->u.serial));
backend->u.serial->device = g_strdup(device);
}
#endif
@@ -3458,6 +3592,7 @@ static void qemu_chr_parse_parallel(QemuOpts *opts, ChardevBackend *backend,
return;
}
backend->u.parallel = g_new0(ChardevHostdev, 1);
+ qemu_chr_parse_common(opts, qapi_ChardevHostdev_base(backend->u.parallel));
backend->u.parallel->device = g_strdup(device);
}
#endif
@@ -3472,6 +3607,7 @@ static void qemu_chr_parse_pipe(QemuOpts *opts, ChardevBackend *backend,
return;
}
backend->u.pipe = g_new0(ChardevHostdev, 1);
+ qemu_chr_parse_common(opts, qapi_ChardevHostdev_base(backend->u.pipe));
backend->u.pipe->device = g_strdup(device);
}
@@ -3481,6 +3617,7 @@ static void qemu_chr_parse_ringbuf(QemuOpts *opts, ChardevBackend *backend,
int val;
backend->u.ringbuf = g_new0(ChardevRingbuf, 1);
+ qemu_chr_parse_common(opts, qapi_ChardevRingbuf_base(backend->u.ringbuf));
val = qemu_opt_get_size(opts, "size", 0);
if (val != 0) {
@@ -3499,6 +3636,7 @@ static void qemu_chr_parse_mux(QemuOpts *opts, ChardevBackend *backend,
return;
}
backend->u.mux = g_new0(ChardevMux, 1);
+ qemu_chr_parse_common(opts, qapi_ChardevMux_base(backend->u.mux));
backend->u.mux->chardev = g_strdup(chardev);
}
@@ -3527,6 +3665,7 @@ static void qemu_chr_parse_socket(QemuOpts *opts, ChardevBackend *backend,
}
backend->u.socket = g_new0(ChardevSocket, 1);
+ qemu_chr_parse_common(opts, qapi_ChardevSocket_base(backend->u.socket));
backend->u.socket->has_nodelay = true;
backend->u.socket->nodelay = do_nodelay;
@@ -3588,6 +3727,7 @@ static void qemu_chr_parse_udp(QemuOpts *opts, ChardevBackend *backend,
}
backend->u.udp = g_new0(ChardevUdp, 1);
+ qemu_chr_parse_common(opts, qapi_ChardevUdp_base(backend->u.udp));
addr = g_new0(SocketAddress, 1);
addr->type = SOCKET_ADDRESS_KIND_INET;
@@ -3687,7 +3827,12 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
error_propagate(errp, local_err);
goto qapi_out;
}
+ } else {
+ ChardevCommon *cc = g_new0(ChardevCommon, 1);
+ qemu_chr_parse_common(opts, cc);
+ backend->u.data = cc;
}
+
ret = qmp_chardev_add(bid ? bid : id, backend, errp);
if (!ret) {
goto qapi_out;
@@ -3819,17 +3964,25 @@ void qemu_chr_fe_release(CharDriverState *s)
s->avail_connections++;
}
-void qemu_chr_free(CharDriverState *chr)
+static void qemu_chr_free_common(CharDriverState *chr)
{
- if (chr->chr_close) {
- chr->chr_close(chr);
- }
g_free(chr->filename);
g_free(chr->label);
qemu_opts_del(chr->opts);
+ if (chr->logfd != -1) {
+ close(chr->logfd);
+ }
g_free(chr);
}
+void qemu_chr_free(CharDriverState *chr)
+{
+ if (chr->chr_close) {
+ chr->chr_close(chr);
+ }
+ qemu_chr_free_common(chr);
+}
+
void qemu_chr_delete(CharDriverState *chr)
{
QTAILQ_REMOVE(&chardevs, chr, next);
@@ -3982,6 +4135,12 @@ QemuOptsList qemu_chardev_opts = {
},{
.name = "append",
.type = QEMU_OPT_BOOL,
+ },{
+ .name = "logfile",
+ .type = QEMU_OPT_STRING,
+ },{
+ .name = "logappend",
+ .type = QEMU_OPT_BOOL,
},
{ /* end of list */ }
},
@@ -3995,6 +4154,7 @@ static CharDriverState *qmp_chardev_open_file(const char *id,
Error **errp)
{
ChardevFile *file = backend->u.file;
+ ChardevCommon *common = qapi_ChardevFile_base(backend->u.file);
HANDLE out;
if (file->has_in) {
@@ -4008,7 +4168,7 @@ static CharDriverState *qmp_chardev_open_file(const char *id,
error_setg(errp, "open %s failed", file->out);
return NULL;
}
- return qemu_chr_open_win_file(out);
+ return qemu_chr_open_win_file(out, common, errp);
}
static CharDriverState *qmp_chardev_open_serial(const char *id,
@@ -4017,7 +4177,8 @@ static CharDriverState *qmp_chardev_open_serial(const char *id,
Error **errp)
{
ChardevHostdev *serial = backend->u.serial;
- return qemu_chr_open_win_path(serial->device, errp);
+ ChardevCommon *common = qapi_ChardevHostdev_base(backend->u.serial);
+ return qemu_chr_open_win_path(serial->device, common, errp);
}
#else /* WIN32 */
@@ -4040,6 +4201,7 @@ static CharDriverState *qmp_chardev_open_file(const char *id,
Error **errp)
{
ChardevFile *file = backend->u.file;
+ ChardevCommon *common = qapi_ChardevFile_base(backend->u.file);
int flags, in = -1, out;
flags = O_WRONLY | O_CREAT | O_BINARY;
@@ -4063,7 +4225,7 @@ static CharDriverState *qmp_chardev_open_file(const char *id,
}
}
- return qemu_chr_open_fd(in, out);
+ return qemu_chr_open_fd(in, out, common, errp);
}
#ifdef HAVE_CHARDEV_SERIAL
@@ -4073,6 +4235,7 @@ static CharDriverState *qmp_chardev_open_serial(const char *id,
Error **errp)
{
ChardevHostdev *serial = backend->u.serial;
+ ChardevCommon *common = qapi_ChardevHostdev_base(backend->u.serial);
int fd;
fd = qmp_chardev_open_file_source(serial->device, O_RDWR, errp);
@@ -4080,7 +4243,7 @@ static CharDriverState *qmp_chardev_open_serial(const char *id,
return NULL;
}
qemu_set_nonblock(fd);
- return qemu_chr_open_tty_fd(fd);
+ return qemu_chr_open_tty_fd(fd, common, errp);
}
#endif
@@ -4091,13 +4254,14 @@ static CharDriverState *qmp_chardev_open_parallel(const char *id,
Error **errp)
{
ChardevHostdev *parallel = backend->u.parallel;
+ ChardevCommon *common = qapi_ChardevHostdev_base(backend->u.parallel);
int fd;
fd = qmp_chardev_open_file_source(parallel->device, O_RDWR, errp);
if (fd < 0) {
return NULL;
}
- return qemu_chr_open_pp_fd(fd, errp);
+ return qemu_chr_open_pp_fd(fd, common, errp);
}
#endif
@@ -4142,8 +4306,12 @@ static CharDriverState *qmp_chardev_open_socket(const char *id,
bool is_telnet = sock->has_telnet ? sock->telnet : false;
bool is_waitconnect = sock->has_wait ? sock->wait : false;
int64_t reconnect = sock->has_reconnect ? sock->reconnect : 0;
+ ChardevCommon *common = qapi_ChardevSocket_base(backend->u.socket);
- chr = qemu_chr_alloc();
+ chr = qemu_chr_alloc(common, errp);
+ if (!chr) {
+ return NULL;
+ }
s = g_new0(TCPCharDriver, 1);
s->fd = -1;
@@ -4182,8 +4350,7 @@ static CharDriverState *qmp_chardev_open_socket(const char *id,
socket_try_connect(chr);
} else if (!qemu_chr_open_socket_fd(chr, errp)) {
g_free(s);
- g_free(chr->filename);
- g_free(chr);
+ qemu_chr_free_common(chr);
return NULL;
}
@@ -4203,13 +4370,14 @@ static CharDriverState *qmp_chardev_open_udp(const char *id,
Error **errp)
{
ChardevUdp *udp = backend->u.udp;
+ ChardevCommon *common = qapi_ChardevUdp_base(backend->u.udp);
int fd;
fd = socket_dgram(udp->remote, udp->local, errp);
if (fd < 0) {
return NULL;
}
- return qemu_chr_open_udp_fd(fd);
+ return qemu_chr_open_udp_fd(fd, common, errp);
}
ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
diff --git a/qemu-options.hx b/qemu-options.hx
index 215d00d..b4763ba 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2089,40 +2089,43 @@ The general form of a character device option is:
ETEXI
DEF("chardev", HAS_ARG, QEMU_OPTION_chardev,
- "-chardev null,id=id[,mux=on|off]\n"
+ "-chardev null,id=id[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
"-chardev socket,id=id[,host=host],port=port[,to=to][,ipv4][,ipv6][,nodelay][,reconnect=seconds]\n"
- " [,server][,nowait][,telnet][,reconnect=seconds][,mux=on|off] (tcp)\n"
- "-chardev socket,id=id,path=path[,server][,nowait][,telnet][,reconnect=seconds][,mux=on|off] (unix)\n"
+ " [,server][,nowait][,telnet][,reconnect=seconds][,mux=on|off]\n"
+ " [,logfile=PATH][,logappend=on|off] (tcp)\n"
+ "-chardev socket,id=id,path=path[,server][,nowait][,telnet][,reconnect=seconds]\n"
+ " [,mux=on|off][,logfile=PATH][,logappend=on|off] (unix)\n"
"-chardev udp,id=id[,host=host],port=port[,localaddr=localaddr]\n"
" [,localport=localport][,ipv4][,ipv6][,mux=on|off]\n"
- "-chardev msmouse,id=id[,mux=on|off]\n"
+ " [,logfile=PATH][,logappend=on|off]\n"
+ "-chardev msmouse,id=id[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
"-chardev vc,id=id[[,width=width][,height=height]][[,cols=cols][,rows=rows]]\n"
- " [,mux=on|off]\n"
- "-chardev ringbuf,id=id[,size=size]\n"
- "-chardev file,id=id,path=path[,mux=on|off]\n"
- "-chardev pipe,id=id,path=path[,mux=on|off]\n"
+ " [,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
+ "-chardev ringbuf,id=id[,size=size][,logfile=PATH][,logappend=on|off]\n"
+ "-chardev file,id=id,path=path[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
+ "-chardev pipe,id=id,path=path[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
#ifdef _WIN32
- "-chardev console,id=id[,mux=on|off]\n"
- "-chardev serial,id=id,path=path[,mux=on|off]\n"
+ "-chardev console,id=id[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
+ "-chardev serial,id=id,path=path[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
#else
- "-chardev pty,id=id[,mux=on|off]\n"
- "-chardev stdio,id=id[,mux=on|off][,signal=on|off]\n"
+ "-chardev pty,id=id[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
+ "-chardev stdio,id=id[,mux=on|off][,signal=on|off][,logfile=PATH][,logappend=on|off]\n"
#endif
#ifdef CONFIG_BRLAPI
- "-chardev braille,id=id[,mux=on|off]\n"
+ "-chardev braille,id=id[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
#endif
#if defined(__linux__) || defined(__sun__) || defined(__FreeBSD__) \
|| defined(__NetBSD__) || defined(__OpenBSD__) || defined(__DragonFly__)
- "-chardev serial,id=id,path=path[,mux=on|off]\n"
- "-chardev tty,id=id,path=path[,mux=on|off]\n"
+ "-chardev serial,id=id,path=path[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
+ "-chardev tty,id=id,path=path[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
#endif
#if defined(__linux__) || defined(__FreeBSD__) || defined(__DragonFly__)
- "-chardev parallel,id=id,path=path[,mux=on|off]\n"
- "-chardev parport,id=id,path=path[,mux=on|off]\n"
+ "-chardev parallel,id=id,path=path[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
+ "-chardev parport,id=id,path=path[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
#endif
#if defined(CONFIG_SPICE)
- "-chardev spicevmc,id=id,name=name[,debug=debug]\n"
- "-chardev spiceport,id=id,name=name[,debug=debug]\n"
+ "-chardev spicevmc,id=id,name=name[,debug=debug][,logfile=PATH][,logappend=on|off]\n"
+ "-chardev spiceport,id=id,name=name[,debug=debug][,logfile=PATH][,logappend=on|off]\n"
#endif
, QEMU_ARCH_ALL
)
@@ -2158,7 +2161,12 @@ A character device may be used in multiplexing mode by multiple front-ends.
The key sequence of @key{Control-a} and @key{c} will rotate the input focus
between attached front-ends. Specify @option{mux=on} to enable this mode.
-Options to each backend are described below.
+Every backend supports the @option{logfile} option, which supplies the path
+to a file to record all data transmitted via the backend. The @option{logappend}
+option controls whether the log file will be truncated or appended to when
+opened.
+
+Further options to each backend are described below.
@item -chardev null ,id=@var{id}
A void device. This device will not emit any data, and will drop any data it
diff --git a/spice-qemu-char.c b/spice-qemu-char.c
index e70e0f7..8951d7c 100644
--- a/spice-qemu-char.c
+++ b/spice-qemu-char.c
@@ -271,13 +271,18 @@ static void spice_chr_accept_input(struct CharDriverState *chr)
}
static CharDriverState *chr_open(const char *subtype,
- void (*set_fe_open)(struct CharDriverState *, int))
-
+ void (*set_fe_open)(struct CharDriverState *,
+ int),
+ ChardevCommon *backend,
+ Error **errp)
{
CharDriverState *chr;
SpiceCharDriver *s;
- chr = qemu_chr_alloc();
+ chr = qemu_chr_alloc(backend, errp);
+ if (!chr) {
+ return NULL;
+ }
s = g_malloc0(sizeof(SpiceCharDriver));
s->chr = chr;
s->active = false;
@@ -303,6 +308,7 @@ static CharDriverState *qemu_chr_open_spice_vmc(const char *id,
{
const char *type = backend->u.spicevmc->type;
const char **psubtype = spice_server_char_device_recognized_subtypes();
+ ChardevCommon *common = qapi_ChardevSpiceChannel_base(backend->u.spicevmc);
for (; *psubtype != NULL; ++psubtype) {
if (strcmp(type, *psubtype) == 0) {
@@ -315,7 +321,7 @@ static CharDriverState *qemu_chr_open_spice_vmc(const char *id,
return NULL;
}
- return chr_open(type, spice_vmc_set_fe_open);
+ return chr_open(type, spice_vmc_set_fe_open, common, errp);
}
#if SPICE_SERVER_VERSION >= 0x000c02
@@ -325,6 +331,7 @@ static CharDriverState *qemu_chr_open_spice_port(const char *id,
Error **errp)
{
const char *name = backend->u.spiceport->fqdn;
+ ChardevCommon *common = qapi_ChardevSpicePort_base(backend->u.spiceport);
CharDriverState *chr;
SpiceCharDriver *s;
@@ -333,7 +340,10 @@ static CharDriverState *qemu_chr_open_spice_port(const char *id,
return NULL;
}
- chr = chr_open("port", spice_port_set_fe_open);
+ chr = chr_open("port", spice_port_set_fe_open, common, errp);
+ if (!chr) {
+ return NULL;
+ }
s = chr->opaque;
s->sin.portname = g_strdup(name);
diff --git a/ui/console.c b/ui/console.c
index 4b65c34..fe950c6 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -1953,12 +1953,16 @@ static void text_console_do_init(CharDriverState *chr, DisplayState *ds)
static CharDriverState *text_console_init(ChardevVC *vc, Error **errp)
{
+ ChardevCommon *common = qapi_ChardevVC_base(vc);
CharDriverState *chr;
QemuConsole *s;
unsigned width = 0;
unsigned height = 0;
- chr = qemu_chr_alloc();
+ chr = qemu_chr_alloc(common, errp);
+ if (!chr) {
+ return NULL;
+ }
if (vc->has_width) {
width = vc->width;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PULL 14/15] qemu-char: add logfile facility to all chardev backends
2016-01-15 16:04 ` [Qemu-devel] [PULL 14/15] qemu-char: add logfile facility to all chardev backends Paolo Bonzini
@ 2016-01-21 6:16 ` Hervé Poussineau
2016-01-21 8:56 ` Daniel P. Berrange
0 siblings, 1 reply; 30+ messages in thread
From: Hervé Poussineau @ 2016-01-21 6:16 UTC (permalink / raw)
To: qemu-devel, Paolo Bonzini, Daniel P. Berrange"
Hi,
This patch (commit d0d7708ba29cbcc343364a46bff981e0ff88366f) regresses the following command line:
qemu-system-i386 -nodefaults -chardev vc,id=mon0 -mon chardev=mon0
Before:
[nothing is print on console]
After:
QEMU 2.5.50 monitor - type 'help' for more information
(qemu)
In both cases, a working vc is created in the GTK+ UI.
Reverting the commit (and fixing the trivial conflict) makes things work again.
Regards,
Hervé
Le 15/01/2016 17:04, Paolo Bonzini a écrit :
> From: "Daniel P. Berrange" <berrange@redhat.com>
>
> Typically a UNIX guest OS will log boot messages to a serial
> port in addition to any graphical console. An admin user
> may also wish to use the serial port for an interactive
> console. A virtualization management system may wish to
> collect system boot messages by logging the serial port,
> but also wish to allow admins interactive access.
>
> Currently providing such a feature forces the mgmt app
> to either provide 2 separate serial ports, one for
> logging boot messages and one for interactive console
> login, or to proxy all output via a separate service
> that can multiplex the two needs onto one serial port.
> While both are valid approaches, they each have their
> own downsides. The former causes confusion and extra
> setup work for VM admins creating disk images. The latter
> places an extra burden to re-implement much of the QEMU
> chardev backends logic in libvirt or even higher level
> mgmt apps and adds extra hops in the data transfer path.
>
> A simpler approach that is satisfactory for many use
> cases is to allow the QEMU chardev backends to have a
> "logfile" property associated with them.
>
> $QEMU -chardev socket,host=localhost,port=9000,\
> server=on,nowait,id-charserial0,\
> logfile=/var/log/libvirt/qemu/test-serial0.log
> -device isa-serial,chardev=charserial0,id=serial0
>
> This patch introduces a 'ChardevCommon' struct which
> is setup as a base for all the ChardevBackend types.
> Ideally this would be registered directly as a base
> against ChardevBackend, rather than each type, but
> the QAPI generator doesn't allow that since the
> ChardevBackend is a non-discriminated union. The
> ChardevCommon struct provides the optional 'logfile'
> parameter, as well as 'logappend' which controls
> whether QEMU truncates or appends (default truncate).
>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> Message-Id: <1452516281-27519-1-git-send-email-berrange@redhat.com>
> [Call qemu_chr_parse_common if cd->parse is NULL. - Paolo]
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> backends/baum.c | 7 +-
> backends/msmouse.c | 6 +-
> gdbstub.c | 3 +-
> include/sysemu/char.h | 9 +-
> qapi-schema.json | 49 +++++++---
> qemu-char.c | 248 ++++++++++++++++++++++++++++++++++++++++++--------
> qemu-options.hx | 48 ++++++----
> spice-qemu-char.c | 20 +++-
> ui/console.c | 6 +-
> 9 files changed, 313 insertions(+), 83 deletions(-)
>
> diff --git a/backends/baum.c b/backends/baum.c
> index 723c658..ba32b61 100644
> --- a/backends/baum.c
> +++ b/backends/baum.c
> @@ -566,6 +566,7 @@ static CharDriverState *chr_baum_init(const char *id,
> ChardevReturn *ret,
> Error **errp)
> {
> + ChardevCommon *common = qapi_ChardevDummy_base(backend->u.braille);
> BaumDriverState *baum;
> CharDriverState *chr;
> brlapi_handle_t *handle;
> @@ -576,8 +577,12 @@ static CharDriverState *chr_baum_init(const char *id,
> #endif
> int tty;
>
> + chr = qemu_chr_alloc(common, errp);
> + if (!chr) {
> + return NULL;
> + }
> baum = g_malloc0(sizeof(BaumDriverState));
> - baum->chr = chr = qemu_chr_alloc();
> + baum->chr = chr;
>
> chr->opaque = baum;
> chr->chr_write = baum_write;
> diff --git a/backends/msmouse.c b/backends/msmouse.c
> index 0126fa0..476dab5 100644
> --- a/backends/msmouse.c
> +++ b/backends/msmouse.c
> @@ -68,9 +68,13 @@ static CharDriverState *qemu_chr_open_msmouse(const char *id,
> ChardevReturn *ret,
> Error **errp)
> {
> + ChardevCommon *common = qapi_ChardevDummy_base(backend->u.msmouse);
> CharDriverState *chr;
>
> - chr = qemu_chr_alloc();
> + chr = qemu_chr_alloc(common, errp);
> + if (!chr) {
> + return NULL;
> + }
> chr->chr_write = msmouse_chr_write;
> chr->chr_close = msmouse_chr_close;
> chr->explicit_be_open = true;
> diff --git a/gdbstub.c b/gdbstub.c
> index 9c29aa0..1a84c1a 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -1732,6 +1732,7 @@ int gdbserver_start(const char *device)
> char gdbstub_device_name[128];
> CharDriverState *chr = NULL;
> CharDriverState *mon_chr;
> + ChardevCommon common = { 0 };
>
> if (!device)
> return -1;
> @@ -1768,7 +1769,7 @@ int gdbserver_start(const char *device)
> qemu_add_vm_change_state_handler(gdb_vm_state_change, NULL);
>
> /* Initialize a monitor terminal for gdb */
> - mon_chr = qemu_chr_alloc();
> + mon_chr = qemu_chr_alloc(&common, &error_abort);
> mon_chr->chr_write = gdb_monitor_write;
> monitor_init(mon_chr, 0);
> } else {
> diff --git a/include/sysemu/char.h b/include/sysemu/char.h
> index aff193f..598dd2b 100644
> --- a/include/sysemu/char.h
> +++ b/include/sysemu/char.h
> @@ -77,6 +77,7 @@ struct CharDriverState {
> void *opaque;
> char *label;
> char *filename;
> + int logfd;
> int be_open;
> int fe_open;
> int explicit_fe_open;
> @@ -89,13 +90,15 @@ struct CharDriverState {
> };
>
> /**
> - * @qemu_chr_alloc:
> + * qemu_chr_alloc:
> + * @backend: the common backend config
> + * @errp: pointer to a NULL-initialized error object
> *
> * Allocate and initialize a new CharDriverState.
> *
> - * Returns: a newly allocated CharDriverState.
> + * Returns: a newly allocated CharDriverState, or NULL on error.
> */
> -CharDriverState *qemu_chr_alloc(void);
> +CharDriverState *qemu_chr_alloc(ChardevCommon *backend, Error **errp);
>
> /**
> * @qemu_chr_new_from_opts:
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 2e31733..dabb5ce 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -3093,6 +3093,21 @@
> ##
> { 'command': 'screendump', 'data': {'filename': 'str'} }
>
> +
> +##
> +# @ChardevCommon:
> +#
> +# Configuration shared across all chardev backends
> +#
> +# @logfile: #optional The name of a logfile to save output
> +# @logappend: #optional true to append instead of truncate
> +# (default to false to truncate)
> +#
> +# Since: 2.6
> +##
> +{ 'struct': 'ChardevCommon', 'data': { '*logfile': 'str',
> + '*logappend': 'bool' } }
> +
> ##
> # @ChardevFile:
> #
> @@ -3107,7 +3122,8 @@
> ##
> { 'struct': 'ChardevFile', 'data': { '*in' : 'str',
> 'out' : 'str',
> - '*append': 'bool' } }
> + '*append': 'bool' },
> + 'base': 'ChardevCommon' }
>
> ##
> # @ChardevHostdev:
> @@ -3120,7 +3136,8 @@
> #
> # Since: 1.4
> ##
> -{ 'struct': 'ChardevHostdev', 'data': { 'device' : 'str' } }
> +{ 'struct': 'ChardevHostdev', 'data': { 'device' : 'str' },
> + 'base': 'ChardevCommon' }
>
> ##
> # @ChardevSocket:
> @@ -3147,7 +3164,8 @@
> '*wait' : 'bool',
> '*nodelay' : 'bool',
> '*telnet' : 'bool',
> - '*reconnect' : 'int' } }
> + '*reconnect' : 'int' },
> + 'base': 'ChardevCommon' }
>
> ##
> # @ChardevUdp:
> @@ -3160,7 +3178,8 @@
> # Since: 1.5
> ##
> { 'struct': 'ChardevUdp', 'data': { 'remote' : 'SocketAddress',
> - '*local' : 'SocketAddress' } }
> + '*local' : 'SocketAddress' },
> + 'base': 'ChardevCommon' }
>
> ##
> # @ChardevMux:
> @@ -3171,7 +3190,8 @@
> #
> # Since: 1.5
> ##
> -{ 'struct': 'ChardevMux', 'data': { 'chardev' : 'str' } }
> +{ 'struct': 'ChardevMux', 'data': { 'chardev' : 'str' },
> + 'base': 'ChardevCommon' }
>
> ##
> # @ChardevStdio:
> @@ -3184,7 +3204,9 @@
> #
> # Since: 1.5
> ##
> -{ 'struct': 'ChardevStdio', 'data': { '*signal' : 'bool' } }
> +{ 'struct': 'ChardevStdio', 'data': { '*signal' : 'bool' },
> + 'base': 'ChardevCommon' }
> +
>
> ##
> # @ChardevSpiceChannel:
> @@ -3195,7 +3217,8 @@
> #
> # Since: 1.5
> ##
> -{ 'struct': 'ChardevSpiceChannel', 'data': { 'type' : 'str' } }
> +{ 'struct': 'ChardevSpiceChannel', 'data': { 'type' : 'str' },
> + 'base': 'ChardevCommon' }
>
> ##
> # @ChardevSpicePort:
> @@ -3206,7 +3229,8 @@
> #
> # Since: 1.5
> ##
> -{ 'struct': 'ChardevSpicePort', 'data': { 'fqdn' : 'str' } }
> +{ 'struct': 'ChardevSpicePort', 'data': { 'fqdn' : 'str' },
> + 'base': 'ChardevCommon' }
>
> ##
> # @ChardevVC:
> @@ -3223,7 +3247,8 @@
> { 'struct': 'ChardevVC', 'data': { '*width' : 'int',
> '*height' : 'int',
> '*cols' : 'int',
> - '*rows' : 'int' } }
> + '*rows' : 'int' },
> + 'base': 'ChardevCommon' }
>
> ##
> # @ChardevRingbuf:
> @@ -3234,7 +3259,8 @@
> #
> # Since: 1.5
> ##
> -{ 'struct': 'ChardevRingbuf', 'data': { '*size' : 'int' } }
> +{ 'struct': 'ChardevRingbuf', 'data': { '*size' : 'int' },
> + 'base': 'ChardevCommon' }
>
> ##
> # @ChardevBackend:
> @@ -3243,7 +3269,8 @@
> #
> # Since: 1.4 (testdev since 2.2)
> ##
> -{ 'struct': 'ChardevDummy', 'data': { } }
> +{ 'struct': 'ChardevDummy', 'data': { },
> + 'base': 'ChardevCommon' }
>
> { 'union': 'ChardevBackend', 'data': { 'file' : 'ChardevFile',
> 'serial' : 'ChardevHostdev',
> diff --git a/qemu-char.c b/qemu-char.c
> index d7be185..11caa56 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -159,10 +159,33 @@ static int sockaddr_to_str(char *dest, int max_len,
> static QTAILQ_HEAD(CharDriverStateHead, CharDriverState) chardevs =
> QTAILQ_HEAD_INITIALIZER(chardevs);
>
> -CharDriverState *qemu_chr_alloc(void)
> +static void qemu_chr_free_common(CharDriverState *chr);
> +
> +CharDriverState *qemu_chr_alloc(ChardevCommon *backend, Error **errp)
> {
> CharDriverState *chr = g_malloc0(sizeof(CharDriverState));
> qemu_mutex_init(&chr->chr_write_lock);
> +
> + if (backend->has_logfile) {
> + int flags = O_WRONLY | O_CREAT;
> + if (backend->has_logappend &&
> + backend->logappend) {
> + flags |= O_APPEND;
> + } else {
> + flags |= O_TRUNC;
> + }
> + chr->logfd = qemu_open(backend->logfile, flags, 0666);
> + if (chr->logfd < 0) {
> + error_setg_errno(errp, errno,
> + "Unable to open logfile %s",
> + backend->logfile);
> + g_free(chr);
> + return NULL;
> + }
> + } else {
> + chr->logfd = -1;
> + }
> +
> return chr;
> }
>
> @@ -188,12 +211,45 @@ void qemu_chr_be_generic_open(CharDriverState *s)
> qemu_chr_be_event(s, CHR_EVENT_OPENED);
> }
>
> +
> +/* Not reporting errors from writing to logfile, as logs are
> + * defined to be "best effort" only */
> +static void qemu_chr_fe_write_log(CharDriverState *s,
> + const uint8_t *buf, size_t len)
> +{
> + size_t done = 0;
> + ssize_t ret;
> +
> + if (s->logfd < 0) {
> + return;
> + }
> +
> + while (done < len) {
> + do {
> + ret = write(s->logfd, buf + done, len - done);
> + if (ret == -1 && errno == EAGAIN) {
> + g_usleep(100);
> + }
> + } while (ret == -1 && errno == EAGAIN);
> +
> + if (ret <= 0) {
> + return;
> + }
> + done += ret;
> + }
> +}
> +
> int qemu_chr_fe_write(CharDriverState *s, const uint8_t *buf, int len)
> {
> int ret;
>
> qemu_mutex_lock(&s->chr_write_lock);
> ret = s->chr_write(s, buf, len);
> +
> + if (ret > 0) {
> + qemu_chr_fe_write_log(s, buf, ret);
> + }
> +
> qemu_mutex_unlock(&s->chr_write_lock);
> return ret;
> }
> @@ -218,6 +274,10 @@ int qemu_chr_fe_write_all(CharDriverState *s, const uint8_t *buf, int len)
>
> offset += res;
> }
> + if (offset > 0) {
> + qemu_chr_fe_write_log(s, buf, offset);
> + }
> +
> qemu_mutex_unlock(&s->chr_write_lock);
>
> if (res < 0) {
> @@ -365,8 +425,12 @@ static CharDriverState *qemu_chr_open_null(const char *id,
> Error **errp)
> {
> CharDriverState *chr;
> + ChardevCommon *common = qapi_ChardevDummy_base(backend->u.null);
>
> - chr = qemu_chr_alloc();
> + chr = qemu_chr_alloc(common, errp);
> + if (!chr) {
> + return NULL;
> + }
> chr->chr_write = null_chr_write;
> chr->explicit_be_open = true;
> return chr;
> @@ -665,6 +729,7 @@ static CharDriverState *qemu_chr_open_mux(const char *id,
> ChardevMux *mux = backend->u.mux;
> CharDriverState *chr, *drv;
> MuxDriver *d;
> + ChardevCommon *common = qapi_ChardevMux_base(backend->u.mux);
>
> drv = qemu_chr_find(mux->chardev);
> if (drv == NULL) {
> @@ -672,7 +737,10 @@ static CharDriverState *qemu_chr_open_mux(const char *id,
> return NULL;
> }
>
> - chr = qemu_chr_alloc();
> + chr = qemu_chr_alloc(common, errp);
> + if (!chr) {
> + return NULL;
> + }
> d = g_new0(MuxDriver, 1);
>
> chr->opaque = d;
> @@ -975,12 +1043,16 @@ static void fd_chr_close(struct CharDriverState *chr)
> }
>
> /* open a character device to a unix fd */
> -static CharDriverState *qemu_chr_open_fd(int fd_in, int fd_out)
> +static CharDriverState *qemu_chr_open_fd(int fd_in, int fd_out,
> + ChardevCommon *backend, Error **errp)
> {
> CharDriverState *chr;
> FDCharDriver *s;
>
> - chr = qemu_chr_alloc();
> + chr = qemu_chr_alloc(backend, errp);
> + if (!chr) {
> + return NULL;
> + }
> s = g_new0(FDCharDriver, 1);
> s->fd_in = io_channel_from_fd(fd_in);
> s->fd_out = io_channel_from_fd(fd_out);
> @@ -1005,6 +1077,7 @@ static CharDriverState *qemu_chr_open_pipe(const char *id,
> char filename_in[CHR_MAX_FILENAME_SIZE];
> char filename_out[CHR_MAX_FILENAME_SIZE];
> const char *filename = opts->device;
> + ChardevCommon *common = qapi_ChardevHostdev_base(backend->u.pipe);
>
> snprintf(filename_in, CHR_MAX_FILENAME_SIZE, "%s.in", filename);
> snprintf(filename_out, CHR_MAX_FILENAME_SIZE, "%s.out", filename);
> @@ -1021,7 +1094,7 @@ static CharDriverState *qemu_chr_open_pipe(const char *id,
> return NULL;
> }
> }
> - return qemu_chr_open_fd(fd_in, fd_out);
> + return qemu_chr_open_fd(fd_in, fd_out, common, errp);
> }
>
> /* init terminal so that we can grab keys */
> @@ -1081,6 +1154,7 @@ static CharDriverState *qemu_chr_open_stdio(const char *id,
> ChardevStdio *opts = backend->u.stdio;
> CharDriverState *chr;
> struct sigaction act;
> + ChardevCommon *common = qapi_ChardevStdio_base(backend->u.stdio);
>
> if (is_daemonized()) {
> error_setg(errp, "cannot use stdio with -daemonize");
> @@ -1102,7 +1176,7 @@ static CharDriverState *qemu_chr_open_stdio(const char *id,
> act.sa_handler = term_stdio_handler;
> sigaction(SIGCONT, &act, NULL);
>
> - chr = qemu_chr_open_fd(0, 1);
> + chr = qemu_chr_open_fd(0, 1, common, errp);
> chr->chr_close = qemu_chr_close_stdio;
> chr->chr_set_echo = qemu_chr_set_echo_stdio;
> if (opts->has_signal) {
> @@ -1324,6 +1398,7 @@ static CharDriverState *qemu_chr_open_pty(const char *id,
> PtyCharDriver *s;
> int master_fd, slave_fd;
> char pty_name[PATH_MAX];
> + ChardevCommon *common = qapi_ChardevDummy_base(backend->u.pty);
>
> master_fd = qemu_openpty_raw(&slave_fd, pty_name);
> if (master_fd < 0) {
> @@ -1334,7 +1409,11 @@ static CharDriverState *qemu_chr_open_pty(const char *id,
> close(slave_fd);
> qemu_set_nonblock(master_fd);
>
> - chr = qemu_chr_alloc();
> + chr = qemu_chr_alloc(common, errp);
> + if (!chr) {
> + close(master_fd);
> + return NULL;
> + }
>
> chr->filename = g_strdup_printf("pty:%s", pty_name);
> ret->pty = g_strdup(pty_name);
> @@ -1557,12 +1636,14 @@ static void qemu_chr_close_tty(CharDriverState *chr)
> }
> }
>
> -static CharDriverState *qemu_chr_open_tty_fd(int fd)
> +static CharDriverState *qemu_chr_open_tty_fd(int fd,
> + ChardevCommon *backend,
> + Error **errp)
> {
> CharDriverState *chr;
>
> tty_serial_init(fd, 115200, 'N', 8, 1);
> - chr = qemu_chr_open_fd(fd, fd);
> + chr = qemu_chr_open_fd(fd, fd, backend, errp);
> chr->chr_ioctl = tty_serial_ioctl;
> chr->chr_close = qemu_chr_close_tty;
> return chr;
> @@ -1682,7 +1763,9 @@ static void pp_close(CharDriverState *chr)
> qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
> }
>
> -static CharDriverState *qemu_chr_open_pp_fd(int fd, Error **errp)
> +static CharDriverState *qemu_chr_open_pp_fd(int fd,
> + ChardevCommon *backend,
> + Error **errp)
> {
> CharDriverState *chr;
> ParallelCharDriver *drv;
> @@ -1697,7 +1780,10 @@ static CharDriverState *qemu_chr_open_pp_fd(int fd, Error **errp)
> drv->fd = fd;
> drv->mode = IEEE1284_MODE_COMPAT;
>
> - chr = qemu_chr_alloc();
> + chr = qemu_chr_alloc(backend, errp);
> + if (!chr) {
> + return NULL;
> + }
> chr->chr_write = null_chr_write;
> chr->chr_ioctl = pp_ioctl;
> chr->chr_close = pp_close;
> @@ -1748,11 +1834,16 @@ static int pp_ioctl(CharDriverState *chr, int cmd, void *arg)
> return 0;
> }
>
> -static CharDriverState *qemu_chr_open_pp_fd(int fd, Error **errp)
> +static CharDriverState *qemu_chr_open_pp_fd(int fd,
> + ChardevBackend *backend,
> + Error **errp)
> {
> CharDriverState *chr;
>
> - chr = qemu_chr_alloc();
> + chr = qemu_chr_alloc(common, errp);
> + if (!chr) {
> + return NULL;
> + }
> chr->opaque = (void *)(intptr_t)fd;
> chr->chr_write = null_chr_write;
> chr->chr_ioctl = pp_ioctl;
> @@ -1978,12 +2069,16 @@ static int win_chr_poll(void *opaque)
> }
>
> static CharDriverState *qemu_chr_open_win_path(const char *filename,
> + ChardevCommon *backend,
> Error **errp)
> {
> CharDriverState *chr;
> WinCharState *s;
>
> - chr = qemu_chr_alloc();
> + chr = qemu_chr_alloc(backend, errp);
> + if (!chr) {
> + return NULL;
> + }
> s = g_new0(WinCharState, 1);
> chr->opaque = s;
> chr->chr_write = win_chr_write;
> @@ -1991,7 +2086,7 @@ static CharDriverState *qemu_chr_open_win_path(const char *filename,
>
> if (win_chr_init(chr, filename, errp) < 0) {
> g_free(s);
> - g_free(chr);
> + qemu_chr_free_common(chr);
> return NULL;
> }
> return chr;
> @@ -2086,8 +2181,12 @@ static CharDriverState *qemu_chr_open_pipe(const char *id,
> const char *filename = opts->device;
> CharDriverState *chr;
> WinCharState *s;
> + ChardevCommon *common = qapi_ChardevHostdev_base(backend->u.pipe);
>
> - chr = qemu_chr_alloc();
> + chr = qemu_chr_alloc(common, errp);
> + if (!chr) {
> + return NULL;
> + }
> s = g_new0(WinCharState, 1);
> chr->opaque = s;
> chr->chr_write = win_chr_write;
> @@ -2095,18 +2194,23 @@ static CharDriverState *qemu_chr_open_pipe(const char *id,
>
> if (win_chr_pipe_init(chr, filename, errp) < 0) {
> g_free(s);
> - g_free(chr);
> + qemu_chr_free_common(chr);
> return NULL;
> }
> return chr;
> }
>
> -static CharDriverState *qemu_chr_open_win_file(HANDLE fd_out)
> +static CharDriverState *qemu_chr_open_win_file(HANDLE fd_out,
> + ChardevCommon *backend,
> + Error **errp)
> {
> CharDriverState *chr;
> WinCharState *s;
>
> - chr = qemu_chr_alloc();
> + chr = qemu_chr_alloc(backend, errp);
> + if (!chr) {
> + return NULL;
> + }
> s = g_new0(WinCharState, 1);
> s->hcom = fd_out;
> chr->opaque = s;
> @@ -2119,7 +2223,9 @@ static CharDriverState *qemu_chr_open_win_con(const char *id,
> ChardevReturn *ret,
> Error **errp)
> {
> - return qemu_chr_open_win_file(GetStdHandle(STD_OUTPUT_HANDLE));
> + ChardevCommon *common = qapi_ChardevDummy_base(backend->u.console);
> + return qemu_chr_open_win_file(GetStdHandle(STD_OUTPUT_HANDLE),
> + common, errp);
> }
>
> static int win_stdio_write(CharDriverState *chr, const uint8_t *buf, int len)
> @@ -2267,8 +2373,12 @@ static CharDriverState *qemu_chr_open_stdio(const char *id,
> WinStdioCharState *stdio;
> DWORD dwMode;
> int is_console = 0;
> + ChardevCommon *common = qapi_ChardevStdio_base(backend->u.stdio);
>
> - chr = qemu_chr_alloc();
> + chr = qemu_chr_alloc(common, errp);
> + if (!chr) {
> + return NULL;
> + }
> stdio = g_new0(WinStdioCharState, 1);
>
> stdio->hStdIn = GetStdHandle(STD_INPUT_HANDLE);
> @@ -2440,12 +2550,17 @@ static void udp_chr_close(CharDriverState *chr)
> qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
> }
>
> -static CharDriverState *qemu_chr_open_udp_fd(int fd)
> +static CharDriverState *qemu_chr_open_udp_fd(int fd,
> + ChardevCommon *backend,
> + Error **errp)
> {
> CharDriverState *chr = NULL;
> NetCharDriver *s = NULL;
>
> - chr = qemu_chr_alloc();
> + chr = qemu_chr_alloc(backend, errp);
> + if (!chr) {
> + return NULL;
> + }
> s = g_new0(NetCharDriver, 1);
>
> s->fd = fd;
> @@ -2856,7 +2971,7 @@ static int tcp_chr_sync_read(CharDriverState *chr, const uint8_t *buf, int len)
> #ifndef _WIN32
> CharDriverState *qemu_chr_open_eventfd(int eventfd)
> {
> - CharDriverState *chr = qemu_chr_open_fd(eventfd, eventfd);
> + CharDriverState *chr = qemu_chr_open_fd(eventfd, eventfd, NULL, NULL);
>
> if (chr) {
> chr->avail_connections = 1;
> @@ -3138,10 +3253,14 @@ static CharDriverState *qemu_chr_open_ringbuf(const char *id,
> Error **errp)
> {
> ChardevRingbuf *opts = backend->u.ringbuf;
> + ChardevCommon *common = qapi_ChardevRingbuf_base(backend->u.ringbuf);
> CharDriverState *chr;
> RingBufCharDriver *d;
>
> - chr = qemu_chr_alloc();
> + chr = qemu_chr_alloc(common, errp);
> + if (!chr) {
> + return NULL;
> + }
> d = g_malloc(sizeof(*d));
>
> d->size = opts->has_size ? opts->size : 65536;
> @@ -3164,7 +3283,7 @@ static CharDriverState *qemu_chr_open_ringbuf(const char *id,
>
> fail:
> g_free(d);
> - g_free(chr);
> + qemu_chr_free_common(chr);
> return NULL;
> }
>
> @@ -3408,6 +3527,18 @@ fail:
> return NULL;
> }
>
> +static void qemu_chr_parse_common(QemuOpts *opts, ChardevCommon *backend)
> +{
> + const char *logfile = qemu_opt_get(opts, "logfile");
> +
> + backend->has_logfile = logfile != NULL;
> + backend->logfile = logfile ? g_strdup(logfile) : NULL;
> +
> + backend->has_logappend = true;
> + backend->logappend = qemu_opt_get_bool(opts, "logappend", false);
> +}
> +
> +
> static void qemu_chr_parse_file_out(QemuOpts *opts, ChardevBackend *backend,
> Error **errp)
> {
> @@ -3418,6 +3549,7 @@ static void qemu_chr_parse_file_out(QemuOpts *opts, ChardevBackend *backend,
> return;
> }
> backend->u.file = g_new0(ChardevFile, 1);
> + qemu_chr_parse_common(opts, qapi_ChardevFile_base(backend->u.file));
> backend->u.file->out = g_strdup(path);
>
> backend->u.file->has_append = true;
> @@ -3428,6 +3560,7 @@ static void qemu_chr_parse_stdio(QemuOpts *opts, ChardevBackend *backend,
> Error **errp)
> {
> backend->u.stdio = g_new0(ChardevStdio, 1);
> + qemu_chr_parse_common(opts, qapi_ChardevStdio_base(backend->u.stdio));
> backend->u.stdio->has_signal = true;
> backend->u.stdio->signal = qemu_opt_get_bool(opts, "signal", true);
> }
> @@ -3443,6 +3576,7 @@ static void qemu_chr_parse_serial(QemuOpts *opts, ChardevBackend *backend,
> return;
> }
> backend->u.serial = g_new0(ChardevHostdev, 1);
> + qemu_chr_parse_common(opts, qapi_ChardevHostdev_base(backend->u.serial));
> backend->u.serial->device = g_strdup(device);
> }
> #endif
> @@ -3458,6 +3592,7 @@ static void qemu_chr_parse_parallel(QemuOpts *opts, ChardevBackend *backend,
> return;
> }
> backend->u.parallel = g_new0(ChardevHostdev, 1);
> + qemu_chr_parse_common(opts, qapi_ChardevHostdev_base(backend->u.parallel));
> backend->u.parallel->device = g_strdup(device);
> }
> #endif
> @@ -3472,6 +3607,7 @@ static void qemu_chr_parse_pipe(QemuOpts *opts, ChardevBackend *backend,
> return;
> }
> backend->u.pipe = g_new0(ChardevHostdev, 1);
> + qemu_chr_parse_common(opts, qapi_ChardevHostdev_base(backend->u.pipe));
> backend->u.pipe->device = g_strdup(device);
> }
>
> @@ -3481,6 +3617,7 @@ static void qemu_chr_parse_ringbuf(QemuOpts *opts, ChardevBackend *backend,
> int val;
>
> backend->u.ringbuf = g_new0(ChardevRingbuf, 1);
> + qemu_chr_parse_common(opts, qapi_ChardevRingbuf_base(backend->u.ringbuf));
>
> val = qemu_opt_get_size(opts, "size", 0);
> if (val != 0) {
> @@ -3499,6 +3636,7 @@ static void qemu_chr_parse_mux(QemuOpts *opts, ChardevBackend *backend,
> return;
> }
> backend->u.mux = g_new0(ChardevMux, 1);
> + qemu_chr_parse_common(opts, qapi_ChardevMux_base(backend->u.mux));
> backend->u.mux->chardev = g_strdup(chardev);
> }
>
> @@ -3527,6 +3665,7 @@ static void qemu_chr_parse_socket(QemuOpts *opts, ChardevBackend *backend,
> }
>
> backend->u.socket = g_new0(ChardevSocket, 1);
> + qemu_chr_parse_common(opts, qapi_ChardevSocket_base(backend->u.socket));
>
> backend->u.socket->has_nodelay = true;
> backend->u.socket->nodelay = do_nodelay;
> @@ -3588,6 +3727,7 @@ static void qemu_chr_parse_udp(QemuOpts *opts, ChardevBackend *backend,
> }
>
> backend->u.udp = g_new0(ChardevUdp, 1);
> + qemu_chr_parse_common(opts, qapi_ChardevUdp_base(backend->u.udp));
>
> addr = g_new0(SocketAddress, 1);
> addr->type = SOCKET_ADDRESS_KIND_INET;
> @@ -3687,7 +3827,12 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
> error_propagate(errp, local_err);
> goto qapi_out;
> }
> + } else {
> + ChardevCommon *cc = g_new0(ChardevCommon, 1);
> + qemu_chr_parse_common(opts, cc);
> + backend->u.data = cc;
> }
> +
> ret = qmp_chardev_add(bid ? bid : id, backend, errp);
> if (!ret) {
> goto qapi_out;
> @@ -3819,17 +3964,25 @@ void qemu_chr_fe_release(CharDriverState *s)
> s->avail_connections++;
> }
>
> -void qemu_chr_free(CharDriverState *chr)
> +static void qemu_chr_free_common(CharDriverState *chr)
> {
> - if (chr->chr_close) {
> - chr->chr_close(chr);
> - }
> g_free(chr->filename);
> g_free(chr->label);
> qemu_opts_del(chr->opts);
> + if (chr->logfd != -1) {
> + close(chr->logfd);
> + }
> g_free(chr);
> }
>
> +void qemu_chr_free(CharDriverState *chr)
> +{
> + if (chr->chr_close) {
> + chr->chr_close(chr);
> + }
> + qemu_chr_free_common(chr);
> +}
> +
> void qemu_chr_delete(CharDriverState *chr)
> {
> QTAILQ_REMOVE(&chardevs, chr, next);
> @@ -3982,6 +4135,12 @@ QemuOptsList qemu_chardev_opts = {
> },{
> .name = "append",
> .type = QEMU_OPT_BOOL,
> + },{
> + .name = "logfile",
> + .type = QEMU_OPT_STRING,
> + },{
> + .name = "logappend",
> + .type = QEMU_OPT_BOOL,
> },
> { /* end of list */ }
> },
> @@ -3995,6 +4154,7 @@ static CharDriverState *qmp_chardev_open_file(const char *id,
> Error **errp)
> {
> ChardevFile *file = backend->u.file;
> + ChardevCommon *common = qapi_ChardevFile_base(backend->u.file);
> HANDLE out;
>
> if (file->has_in) {
> @@ -4008,7 +4168,7 @@ static CharDriverState *qmp_chardev_open_file(const char *id,
> error_setg(errp, "open %s failed", file->out);
> return NULL;
> }
> - return qemu_chr_open_win_file(out);
> + return qemu_chr_open_win_file(out, common, errp);
> }
>
> static CharDriverState *qmp_chardev_open_serial(const char *id,
> @@ -4017,7 +4177,8 @@ static CharDriverState *qmp_chardev_open_serial(const char *id,
> Error **errp)
> {
> ChardevHostdev *serial = backend->u.serial;
> - return qemu_chr_open_win_path(serial->device, errp);
> + ChardevCommon *common = qapi_ChardevHostdev_base(backend->u.serial);
> + return qemu_chr_open_win_path(serial->device, common, errp);
> }
>
> #else /* WIN32 */
> @@ -4040,6 +4201,7 @@ static CharDriverState *qmp_chardev_open_file(const char *id,
> Error **errp)
> {
> ChardevFile *file = backend->u.file;
> + ChardevCommon *common = qapi_ChardevFile_base(backend->u.file);
> int flags, in = -1, out;
>
> flags = O_WRONLY | O_CREAT | O_BINARY;
> @@ -4063,7 +4225,7 @@ static CharDriverState *qmp_chardev_open_file(const char *id,
> }
> }
>
> - return qemu_chr_open_fd(in, out);
> + return qemu_chr_open_fd(in, out, common, errp);
> }
>
> #ifdef HAVE_CHARDEV_SERIAL
> @@ -4073,6 +4235,7 @@ static CharDriverState *qmp_chardev_open_serial(const char *id,
> Error **errp)
> {
> ChardevHostdev *serial = backend->u.serial;
> + ChardevCommon *common = qapi_ChardevHostdev_base(backend->u.serial);
> int fd;
>
> fd = qmp_chardev_open_file_source(serial->device, O_RDWR, errp);
> @@ -4080,7 +4243,7 @@ static CharDriverState *qmp_chardev_open_serial(const char *id,
> return NULL;
> }
> qemu_set_nonblock(fd);
> - return qemu_chr_open_tty_fd(fd);
> + return qemu_chr_open_tty_fd(fd, common, errp);
> }
> #endif
>
> @@ -4091,13 +4254,14 @@ static CharDriverState *qmp_chardev_open_parallel(const char *id,
> Error **errp)
> {
> ChardevHostdev *parallel = backend->u.parallel;
> + ChardevCommon *common = qapi_ChardevHostdev_base(backend->u.parallel);
> int fd;
>
> fd = qmp_chardev_open_file_source(parallel->device, O_RDWR, errp);
> if (fd < 0) {
> return NULL;
> }
> - return qemu_chr_open_pp_fd(fd, errp);
> + return qemu_chr_open_pp_fd(fd, common, errp);
> }
> #endif
>
> @@ -4142,8 +4306,12 @@ static CharDriverState *qmp_chardev_open_socket(const char *id,
> bool is_telnet = sock->has_telnet ? sock->telnet : false;
> bool is_waitconnect = sock->has_wait ? sock->wait : false;
> int64_t reconnect = sock->has_reconnect ? sock->reconnect : 0;
> + ChardevCommon *common = qapi_ChardevSocket_base(backend->u.socket);
>
> - chr = qemu_chr_alloc();
> + chr = qemu_chr_alloc(common, errp);
> + if (!chr) {
> + return NULL;
> + }
> s = g_new0(TCPCharDriver, 1);
>
> s->fd = -1;
> @@ -4182,8 +4350,7 @@ static CharDriverState *qmp_chardev_open_socket(const char *id,
> socket_try_connect(chr);
> } else if (!qemu_chr_open_socket_fd(chr, errp)) {
> g_free(s);
> - g_free(chr->filename);
> - g_free(chr);
> + qemu_chr_free_common(chr);
> return NULL;
> }
>
> @@ -4203,13 +4370,14 @@ static CharDriverState *qmp_chardev_open_udp(const char *id,
> Error **errp)
> {
> ChardevUdp *udp = backend->u.udp;
> + ChardevCommon *common = qapi_ChardevUdp_base(backend->u.udp);
> int fd;
>
> fd = socket_dgram(udp->remote, udp->local, errp);
> if (fd < 0) {
> return NULL;
> }
> - return qemu_chr_open_udp_fd(fd);
> + return qemu_chr_open_udp_fd(fd, common, errp);
> }
>
> ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 215d00d..b4763ba 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -2089,40 +2089,43 @@ The general form of a character device option is:
> ETEXI
>
> DEF("chardev", HAS_ARG, QEMU_OPTION_chardev,
> - "-chardev null,id=id[,mux=on|off]\n"
> + "-chardev null,id=id[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
> "-chardev socket,id=id[,host=host],port=port[,to=to][,ipv4][,ipv6][,nodelay][,reconnect=seconds]\n"
> - " [,server][,nowait][,telnet][,reconnect=seconds][,mux=on|off] (tcp)\n"
> - "-chardev socket,id=id,path=path[,server][,nowait][,telnet][,reconnect=seconds][,mux=on|off] (unix)\n"
> + " [,server][,nowait][,telnet][,reconnect=seconds][,mux=on|off]\n"
> + " [,logfile=PATH][,logappend=on|off] (tcp)\n"
> + "-chardev socket,id=id,path=path[,server][,nowait][,telnet][,reconnect=seconds]\n"
> + " [,mux=on|off][,logfile=PATH][,logappend=on|off] (unix)\n"
> "-chardev udp,id=id[,host=host],port=port[,localaddr=localaddr]\n"
> " [,localport=localport][,ipv4][,ipv6][,mux=on|off]\n"
> - "-chardev msmouse,id=id[,mux=on|off]\n"
> + " [,logfile=PATH][,logappend=on|off]\n"
> + "-chardev msmouse,id=id[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
> "-chardev vc,id=id[[,width=width][,height=height]][[,cols=cols][,rows=rows]]\n"
> - " [,mux=on|off]\n"
> - "-chardev ringbuf,id=id[,size=size]\n"
> - "-chardev file,id=id,path=path[,mux=on|off]\n"
> - "-chardev pipe,id=id,path=path[,mux=on|off]\n"
> + " [,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
> + "-chardev ringbuf,id=id[,size=size][,logfile=PATH][,logappend=on|off]\n"
> + "-chardev file,id=id,path=path[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
> + "-chardev pipe,id=id,path=path[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
> #ifdef _WIN32
> - "-chardev console,id=id[,mux=on|off]\n"
> - "-chardev serial,id=id,path=path[,mux=on|off]\n"
> + "-chardev console,id=id[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
> + "-chardev serial,id=id,path=path[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
> #else
> - "-chardev pty,id=id[,mux=on|off]\n"
> - "-chardev stdio,id=id[,mux=on|off][,signal=on|off]\n"
> + "-chardev pty,id=id[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
> + "-chardev stdio,id=id[,mux=on|off][,signal=on|off][,logfile=PATH][,logappend=on|off]\n"
> #endif
> #ifdef CONFIG_BRLAPI
> - "-chardev braille,id=id[,mux=on|off]\n"
> + "-chardev braille,id=id[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
> #endif
> #if defined(__linux__) || defined(__sun__) || defined(__FreeBSD__) \
> || defined(__NetBSD__) || defined(__OpenBSD__) || defined(__DragonFly__)
> - "-chardev serial,id=id,path=path[,mux=on|off]\n"
> - "-chardev tty,id=id,path=path[,mux=on|off]\n"
> + "-chardev serial,id=id,path=path[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
> + "-chardev tty,id=id,path=path[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
> #endif
> #if defined(__linux__) || defined(__FreeBSD__) || defined(__DragonFly__)
> - "-chardev parallel,id=id,path=path[,mux=on|off]\n"
> - "-chardev parport,id=id,path=path[,mux=on|off]\n"
> + "-chardev parallel,id=id,path=path[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
> + "-chardev parport,id=id,path=path[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
> #endif
> #if defined(CONFIG_SPICE)
> - "-chardev spicevmc,id=id,name=name[,debug=debug]\n"
> - "-chardev spiceport,id=id,name=name[,debug=debug]\n"
> + "-chardev spicevmc,id=id,name=name[,debug=debug][,logfile=PATH][,logappend=on|off]\n"
> + "-chardev spiceport,id=id,name=name[,debug=debug][,logfile=PATH][,logappend=on|off]\n"
> #endif
> , QEMU_ARCH_ALL
> )
> @@ -2158,7 +2161,12 @@ A character device may be used in multiplexing mode by multiple front-ends.
> The key sequence of @key{Control-a} and @key{c} will rotate the input focus
> between attached front-ends. Specify @option{mux=on} to enable this mode.
>
> -Options to each backend are described below.
> +Every backend supports the @option{logfile} option, which supplies the path
> +to a file to record all data transmitted via the backend. The @option{logappend}
> +option controls whether the log file will be truncated or appended to when
> +opened.
> +
> +Further options to each backend are described below.
>
> @item -chardev null ,id=@var{id}
> A void device. This device will not emit any data, and will drop any data it
> diff --git a/spice-qemu-char.c b/spice-qemu-char.c
> index e70e0f7..8951d7c 100644
> --- a/spice-qemu-char.c
> +++ b/spice-qemu-char.c
> @@ -271,13 +271,18 @@ static void spice_chr_accept_input(struct CharDriverState *chr)
> }
>
> static CharDriverState *chr_open(const char *subtype,
> - void (*set_fe_open)(struct CharDriverState *, int))
> -
> + void (*set_fe_open)(struct CharDriverState *,
> + int),
> + ChardevCommon *backend,
> + Error **errp)
> {
> CharDriverState *chr;
> SpiceCharDriver *s;
>
> - chr = qemu_chr_alloc();
> + chr = qemu_chr_alloc(backend, errp);
> + if (!chr) {
> + return NULL;
> + }
> s = g_malloc0(sizeof(SpiceCharDriver));
> s->chr = chr;
> s->active = false;
> @@ -303,6 +308,7 @@ static CharDriverState *qemu_chr_open_spice_vmc(const char *id,
> {
> const char *type = backend->u.spicevmc->type;
> const char **psubtype = spice_server_char_device_recognized_subtypes();
> + ChardevCommon *common = qapi_ChardevSpiceChannel_base(backend->u.spicevmc);
>
> for (; *psubtype != NULL; ++psubtype) {
> if (strcmp(type, *psubtype) == 0) {
> @@ -315,7 +321,7 @@ static CharDriverState *qemu_chr_open_spice_vmc(const char *id,
> return NULL;
> }
>
> - return chr_open(type, spice_vmc_set_fe_open);
> + return chr_open(type, spice_vmc_set_fe_open, common, errp);
> }
>
> #if SPICE_SERVER_VERSION >= 0x000c02
> @@ -325,6 +331,7 @@ static CharDriverState *qemu_chr_open_spice_port(const char *id,
> Error **errp)
> {
> const char *name = backend->u.spiceport->fqdn;
> + ChardevCommon *common = qapi_ChardevSpicePort_base(backend->u.spiceport);
> CharDriverState *chr;
> SpiceCharDriver *s;
>
> @@ -333,7 +340,10 @@ static CharDriverState *qemu_chr_open_spice_port(const char *id,
> return NULL;
> }
>
> - chr = chr_open("port", spice_port_set_fe_open);
> + chr = chr_open("port", spice_port_set_fe_open, common, errp);
> + if (!chr) {
> + return NULL;
> + }
> s = chr->opaque;
> s->sin.portname = g_strdup(name);
>
> diff --git a/ui/console.c b/ui/console.c
> index 4b65c34..fe950c6 100644
> --- a/ui/console.c
> +++ b/ui/console.c
> @@ -1953,12 +1953,16 @@ static void text_console_do_init(CharDriverState *chr, DisplayState *ds)
>
> static CharDriverState *text_console_init(ChardevVC *vc, Error **errp)
> {
> + ChardevCommon *common = qapi_ChardevVC_base(vc);
> CharDriverState *chr;
> QemuConsole *s;
> unsigned width = 0;
> unsigned height = 0;
>
> - chr = qemu_chr_alloc();
> + chr = qemu_chr_alloc(common, errp);
> + if (!chr) {
> + return NULL;
> + }
>
> if (vc->has_width) {
> width = vc->width;
>
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PULL 14/15] qemu-char: add logfile facility to all chardev backends
2016-01-21 6:16 ` Hervé Poussineau
@ 2016-01-21 8:56 ` Daniel P. Berrange
2016-02-12 16:49 ` Markus Armbruster
0 siblings, 1 reply; 30+ messages in thread
From: Daniel P. Berrange @ 2016-01-21 8:56 UTC (permalink / raw)
To: Hervé Poussineau; +Cc: Paolo Bonzini, qemu-devel
On Thu, Jan 21, 2016 at 07:16:25AM +0100, Hervé Poussineau wrote:
> Hi,
>
> This patch (commit d0d7708ba29cbcc343364a46bff981e0ff88366f) regresses the following command line:
> qemu-system-i386 -nodefaults -chardev vc,id=mon0 -mon chardev=mon0
>
> Before:
> [nothing is print on console]
>
> After:
> QEMU 2.5.50 monitor - type 'help' for more information
> (qemu)
>
> In both cases, a working vc is created in the GTK+ UI.
>
> Reverting the commit (and fixing the trivial conflict) makes things
> work again.
Thanks for the heads-up, I'll investigate and submit a fix
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PULL 14/15] qemu-char: add logfile facility to all chardev backends
2016-01-21 8:56 ` Daniel P. Berrange
@ 2016-02-12 16:49 ` Markus Armbruster
2016-02-12 16:54 ` Daniel P. Berrange
0 siblings, 1 reply; 30+ messages in thread
From: Markus Armbruster @ 2016-02-12 16:49 UTC (permalink / raw)
To: Daniel P. Berrange; +Cc: Paolo Bonzini, Hervé Poussineau, qemu-devel
"Daniel P. Berrange" <berrange@redhat.com> writes:
> On Thu, Jan 21, 2016 at 07:16:25AM +0100, Hervé Poussineau wrote:
>> Hi,
>>
>> This patch (commit d0d7708ba29cbcc343364a46bff981e0ff88366f) regresses the following command line:
>> qemu-system-i386 -nodefaults -chardev vc,id=mon0 -mon chardev=mon0
>>
>> Before:
>> [nothing is print on console]
>>
>> After:
>> QEMU 2.5.50 monitor - type 'help' for more information
>> (qemu)
>>
>> In both cases, a working vc is created in the GTK+ UI.
>>
>> Reverting the commit (and fixing the trivial conflict) makes things
>> work again.
>
> Thanks for the heads-up, I'll investigate and submit a fix
For the record, it also breaks ivshmem-test when the slow tests are
included. I'd be willing to test your fix; got a pointer for me?
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PULL 14/15] qemu-char: add logfile facility to all chardev backends
2016-02-12 16:49 ` Markus Armbruster
@ 2016-02-12 16:54 ` Daniel P. Berrange
2016-02-12 17:12 ` Markus Armbruster
0 siblings, 1 reply; 30+ messages in thread
From: Daniel P. Berrange @ 2016-02-12 16:54 UTC (permalink / raw)
To: Markus Armbruster; +Cc: Paolo Bonzini, Hervé Poussineau, qemu-devel
On Fri, Feb 12, 2016 at 05:49:38PM +0100, Markus Armbruster wrote:
> "Daniel P. Berrange" <berrange@redhat.com> writes:
>
> > On Thu, Jan 21, 2016 at 07:16:25AM +0100, Hervé Poussineau wrote:
> >> Hi,
> >>
> >> This patch (commit d0d7708ba29cbcc343364a46bff981e0ff88366f) regresses the following command line:
> >> qemu-system-i386 -nodefaults -chardev vc,id=mon0 -mon chardev=mon0
> >>
> >> Before:
> >> [nothing is print on console]
> >>
> >> After:
> >> QEMU 2.5.50 monitor - type 'help' for more information
> >> (qemu)
> >>
> >> In both cases, a working vc is created in the GTK+ UI.
> >>
> >> Reverting the commit (and fixing the trivial conflict) makes things
> >> work again.
> >
> > Thanks for the heads-up, I'll investigate and submit a fix
>
> For the record, it also breaks ivshmem-test when the slow tests are
> included. I'd be willing to test your fix; got a pointer for me?
https://lists.gnu.org/archive/html/qemu-devel/2016-02/msg02738.html
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PULL 14/15] qemu-char: add logfile facility to all chardev backends
2016-02-12 16:54 ` Daniel P. Berrange
@ 2016-02-12 17:12 ` Markus Armbruster
2016-02-12 18:04 ` Daniel P. Berrange
0 siblings, 1 reply; 30+ messages in thread
From: Markus Armbruster @ 2016-02-12 17:12 UTC (permalink / raw)
To: Daniel P. Berrange; +Cc: Paolo Bonzini, Hervé Poussineau, qemu-devel
"Daniel P. Berrange" <berrange@redhat.com> writes:
> On Fri, Feb 12, 2016 at 05:49:38PM +0100, Markus Armbruster wrote:
>> "Daniel P. Berrange" <berrange@redhat.com> writes:
>>
>> > On Thu, Jan 21, 2016 at 07:16:25AM +0100, Hervé Poussineau wrote:
>> >> Hi,
>> >>
>> >> This patch (commit d0d7708ba29cbcc343364a46bff981e0ff88366f) regresses the following command line:
>> >> qemu-system-i386 -nodefaults -chardev vc,id=mon0 -mon chardev=mon0
>> >>
>> >> Before:
>> >> [nothing is print on console]
>> >>
>> >> After:
>> >> QEMU 2.5.50 monitor - type 'help' for more information
>> >> (qemu)
>> >>
>> >> In both cases, a working vc is created in the GTK+ UI.
>> >>
>> >> Reverting the commit (and fixing the trivial conflict) makes things
>> >> work again.
>> >
>> > Thanks for the heads-up, I'll investigate and submit a fix
>>
>> For the record, it also breaks ivshmem-test when the slow tests are
>> included. I'd be willing to test your fix; got a pointer for me?
>
> https://lists.gnu.org/archive/html/qemu-devel/2016-02/msg02738.html
No luck. Please try the following reproducer:
$ make tests/ivshmem-test
$ QTEST_QEMU_BINARY='x86_64-softmmu/qemu-system-x86_64' QTEST_QEMU_IMG=qemu-img MALLOC_PERTURB_=${MALLOC_PERTURB_:-$((RANDOM % 255 + 1))} gtester -k --verbose -m slow tests/ivshmem-test
You can instead run make check SPEED=slow, but that runs many more
tests.
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PULL 14/15] qemu-char: add logfile facility to all chardev backends
2016-02-12 17:12 ` Markus Armbruster
@ 2016-02-12 18:04 ` Daniel P. Berrange
2016-02-12 21:53 ` Daniel P. Berrange
0 siblings, 1 reply; 30+ messages in thread
From: Daniel P. Berrange @ 2016-02-12 18:04 UTC (permalink / raw)
To: Markus Armbruster; +Cc: Paolo Bonzini, Hervé Poussineau, qemu-devel
On Fri, Feb 12, 2016 at 06:12:46PM +0100, Markus Armbruster wrote:
> "Daniel P. Berrange" <berrange@redhat.com> writes:
>
> > On Fri, Feb 12, 2016 at 05:49:38PM +0100, Markus Armbruster wrote:
> >> "Daniel P. Berrange" <berrange@redhat.com> writes:
> >>
> >> > On Thu, Jan 21, 2016 at 07:16:25AM +0100, Hervé Poussineau wrote:
> >> >> Hi,
> >> >>
> >> >> This patch (commit d0d7708ba29cbcc343364a46bff981e0ff88366f) regresses the following command line:
> >> >> qemu-system-i386 -nodefaults -chardev vc,id=mon0 -mon chardev=mon0
> >> >>
> >> >> Before:
> >> >> [nothing is print on console]
> >> >>
> >> >> After:
> >> >> QEMU 2.5.50 monitor - type 'help' for more information
> >> >> (qemu)
> >> >>
> >> >> In both cases, a working vc is created in the GTK+ UI.
> >> >>
> >> >> Reverting the commit (and fixing the trivial conflict) makes things
> >> >> work again.
> >> >
> >> > Thanks for the heads-up, I'll investigate and submit a fix
> >>
> >> For the record, it also breaks ivshmem-test when the slow tests are
> >> included. I'd be willing to test your fix; got a pointer for me?
> >
> > https://lists.gnu.org/archive/html/qemu-devel/2016-02/msg02738.html
>
> No luck. Please try the following reproducer:
>
> $ make tests/ivshmem-test
> $ QTEST_QEMU_BINARY='x86_64-softmmu/qemu-system-x86_64' QTEST_QEMU_IMG=qemu-img MALLOC_PERTURB_=${MALLOC_PERTURB_:-$((RANDOM % 255 + 1))} gtester -k --verbose -m slow tests/ivshmem-test
Yes, I can reproduce that. Will put it on my todo list for next week.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PULL 14/15] qemu-char: add logfile facility to all chardev backends
2016-02-12 18:04 ` Daniel P. Berrange
@ 2016-02-12 21:53 ` Daniel P. Berrange
2016-02-15 8:23 ` Markus Armbruster
0 siblings, 1 reply; 30+ messages in thread
From: Daniel P. Berrange @ 2016-02-12 21:53 UTC (permalink / raw)
To: Markus Armbruster; +Cc: Paolo Bonzini, Hervé Poussineau, qemu-devel
On Fri, Feb 12, 2016 at 06:04:59PM +0000, Daniel P. Berrange wrote:
> On Fri, Feb 12, 2016 at 06:12:46PM +0100, Markus Armbruster wrote:
> > "Daniel P. Berrange" <berrange@redhat.com> writes:
> >
> > > On Fri, Feb 12, 2016 at 05:49:38PM +0100, Markus Armbruster wrote:
> > >> "Daniel P. Berrange" <berrange@redhat.com> writes:
> > >>
> > >> > On Thu, Jan 21, 2016 at 07:16:25AM +0100, Hervé Poussineau wrote:
> > >> >> Hi,
> > >> >>
> > >> >> This patch (commit d0d7708ba29cbcc343364a46bff981e0ff88366f) regresses the following command line:
> > >> >> qemu-system-i386 -nodefaults -chardev vc,id=mon0 -mon chardev=mon0
> > >> >>
> > >> >> Before:
> > >> >> [nothing is print on console]
> > >> >>
> > >> >> After:
> > >> >> QEMU 2.5.50 monitor - type 'help' for more information
> > >> >> (qemu)
> > >> >>
> > >> >> In both cases, a working vc is created in the GTK+ UI.
> > >> >>
> > >> >> Reverting the commit (and fixing the trivial conflict) makes things
> > >> >> work again.
> > >> >
> > >> > Thanks for the heads-up, I'll investigate and submit a fix
> > >>
> > >> For the record, it also breaks ivshmem-test when the slow tests are
> > >> included. I'd be willing to test your fix; got a pointer for me?
> > >
> > > https://lists.gnu.org/archive/html/qemu-devel/2016-02/msg02738.html
> >
> > No luck. Please try the following reproducer:
> >
> > $ make tests/ivshmem-test
> > $ QTEST_QEMU_BINARY='x86_64-softmmu/qemu-system-x86_64' QTEST_QEMU_IMG=qemu-img MALLOC_PERTURB_=${MALLOC_PERTURB_:-$((RANDOM % 255 + 1))} gtester -k --verbose -m slow tests/ivshmem-test
>
> Yes, I can reproduce that. Will put it on my todo list for next week.
So I investigated this, and my commit here is not the cause. My commit
introduced a crash bug due to NULL pointer reference. This was fixed
already in by Marc-Andre in
commit 9940c3236f318949c92099163281d5d23a9fcf4f
Author: Marc-André Lureau <marcandre.lureau@redhat.com>
Date: Mon Dec 21 12:10:13 2015 +0100
ivshmem: use a single eventfd callback, get rid of CharDriver
I bisected the test failure again and found this change to blame:
commit 428c3ece97179557f2753071fb0ca97a03437267
Author: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Date: Wed Jan 13 14:59:09 2016 +0000
fix MSI injection on Xen
which makes more sense, because the test case failure you identified
is specifically with the MSI code in ivshmem
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PULL 14/15] qemu-char: add logfile facility to all chardev backends
2016-02-12 21:53 ` Daniel P. Berrange
@ 2016-02-15 8:23 ` Markus Armbruster
0 siblings, 0 replies; 30+ messages in thread
From: Markus Armbruster @ 2016-02-15 8:23 UTC (permalink / raw)
To: Daniel P. Berrange; +Cc: Paolo Bonzini, Hervé Poussineau, qemu-devel
"Daniel P. Berrange" <berrange@redhat.com> writes:
> On Fri, Feb 12, 2016 at 06:04:59PM +0000, Daniel P. Berrange wrote:
>> On Fri, Feb 12, 2016 at 06:12:46PM +0100, Markus Armbruster wrote:
[...]
>> > >> For the record, it also breaks ivshmem-test when the slow tests are
>> > >> included. I'd be willing to test your fix; got a pointer for me?
>> > >
>> > > https://lists.gnu.org/archive/html/qemu-devel/2016-02/msg02738.html
>> >
>> > No luck. Please try the following reproducer:
>> >
>> > $ make tests/ivshmem-test
>> > $ QTEST_QEMU_BINARY='x86_64-softmmu/qemu-system-x86_64' QTEST_QEMU_IMG=qemu-img MALLOC_PERTURB_=${MALLOC_PERTURB_:-$((RANDOM % 255 + 1))} gtester -k --verbose -m slow tests/ivshmem-test
>>
>> Yes, I can reproduce that. Will put it on my todo list for next week.
>
> So I investigated this, and my commit here is not the cause. My commit
> introduced a crash bug due to NULL pointer reference. This was fixed
> already in by Marc-Andre in
>
>
> commit 9940c3236f318949c92099163281d5d23a9fcf4f
> Author: Marc-André Lureau <marcandre.lureau@redhat.com>
> Date: Mon Dec 21 12:10:13 2015 +0100
>
> ivshmem: use a single eventfd callback, get rid of CharDriver
>
>
> I bisected the test failure again and found this change to blame:
>
> commit 428c3ece97179557f2753071fb0ca97a03437267
> Author: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Date: Wed Jan 13 14:59:09 2016 +0000
>
> fix MSI injection on Xen
>
> which makes more sense, because the test case failure you identified
> is specifically with the MSI code in ivshmem
Thanks, and sorry for my sloppy bisecting.
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [PULL 15/15] qemu-char: do not leak QemuMutex when freeing a character device
2016-01-15 16:04 [Qemu-devel] [PULL 00/15] NBD, chardev, SCSI patches for 2015-01-15 Paolo Bonzini
` (14 preceding siblings ...)
2016-01-15 16:04 ` [Qemu-devel] [PULL 14/15] qemu-char: add logfile facility to all chardev backends Paolo Bonzini
@ 2016-01-15 16:04 ` Paolo Bonzini
2016-01-15 17:42 ` [Qemu-devel] [PULL 00/15] NBD, chardev, SCSI patches for 2015-01-15 Peter Maydell
16 siblings, 0 replies; 30+ messages in thread
From: Paolo Bonzini @ 2016-01-15 16:04 UTC (permalink / raw)
To: qemu-devel
The leak is only apparent on Win32. On POSIX platforms destroying a
mutex is not necessary.
Reported-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
qemu-char.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/qemu-char.c b/qemu-char.c
index 11caa56..e133f4f 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -3972,6 +3972,7 @@ static void qemu_chr_free_common(CharDriverState *chr)
if (chr->logfd != -1) {
close(chr->logfd);
}
+ qemu_mutex_destroy(&chr->chr_write_lock);
g_free(chr);
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PULL 00/15] NBD, chardev, SCSI patches for 2015-01-15
2016-01-15 16:04 [Qemu-devel] [PULL 00/15] NBD, chardev, SCSI patches for 2015-01-15 Paolo Bonzini
` (15 preceding siblings ...)
2016-01-15 16:04 ` [Qemu-devel] [PULL 15/15] qemu-char: do not leak QemuMutex when freeing a character device Paolo Bonzini
@ 2016-01-15 17:42 ` Peter Maydell
16 siblings, 0 replies; 30+ messages in thread
From: Peter Maydell @ 2016-01-15 17:42 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: QEMU Developers
On 15 January 2016 at 16:04, Paolo Bonzini <pbonzini@redhat.com> wrote:
> The following changes since commit f02ccf53693758b65843264e077f90cf295e7d98:
>
> disas/libvixl: Really suppress gcc 4.6.3 sign-compare warnings (2016-01-14 17:57:51 +0000)
>
> are available in the git repository at:
>
> git://github.com/bonzini/qemu.git tags/for-upstream
>
> for you to fetch changes up to 196ab03442304823ee8e7f46bdca49ea6516ebe5:
>
> qemu-char: do not leak QemuMutex when freeing a character device (2016-01-15 17:03:09 +0100)
>
> ----------------------------------------------------------------
> * qemu-char logfile facility
> * NBD coroutine based negotiation
> * bugfixes
Hi. I'm afraid this fails to build for w32:
target-i386/helper.c: In function ‘x86_cpu_handle_mmu_fault’:
target-i386/helper.c:930: error: ‘PROT_WRITE’ undeclared (first use in
this function)
target-i386/helper.c:930: error: (Each undeclared identifier is
reported only once
target-i386/helper.c:930: error: for each function it appears in.)
Looks like the code intended to use PAGE_WRITE.
thanks
-- PMM
^ permalink raw reply [flat|nested] 30+ messages in thread