Re: [Qemu-devel] [Qemu-block] [PATCH 1/2] vhost-user-blk: prevent using uninitialized vqs
Raphael Norwitz 於 2019-08-23 04:16 寫道： > > Same rational as: e6cc11d64fc998c11a4dfcde8fda3fc33a74d844 > > Of the 3 virtqueues, seabios only sets cmd, leaving ctrl > and event without a physical address. This can cause > vhost_verify_ring_part_mapping to return ENOMEM, causing > the following logs: > > qemu-system-x86_64: Unable to map available ring for ring 0 > qemu-system-x86_64: Verify ring failure on region 0 > > This has already been fixed for vhost scsi devices and was > recently vhost-user scsi devices. This commit fixes it for > vhost-user-blk devices. > > Suggested-by: Phillippe Mathieu-Daude > Signed-off-by: Raphael Norwitz Reviewed-by: yuchenlin Thanks. > > > --- > hw/block/vhost-user-blk.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c > index 0b8c5df..63da9bb 100644 > --- a/hw/block/vhost-user-blk.c > +++ b/hw/block/vhost-user-blk.c > @@ -421,7 +421,7 @@ static void vhost_user_blk_device_realize(DeviceState *dev, Error **errp) > } > > s->inflight = g_new0(struct vhost_inflight, 1); > - s->vqs = g_new(struct vhost_virtqueue, s->num_queues); > + s->vqs = g_new0(struct vhost_virtqueue, s->num_queues); > s->watch = 0; > s->connected = false; > > -- > 1.9.4 > >
Re: [Qemu-devel] [Qemu-block] [PATCH 1/2] vmdk: Fix comment regarding max l1_size coverage
On 2019-04-24 15:49, Sam Eiderman wrote:
Commit b0651b8c246d ("vmdk: Move l1_size check into vmdk_add_extent")
extended the l1_size check from VMDK4 to VMDK3 but did not update the
default coverage in the moved comment.
The previous vmdk4 calculation:
(512 * 1024 * 1024) * 512(l2 entries) * 65536(grain) = 16PB
The added vmdk3 calculation:
(512 * 1024 * 1024) * 4096(l2 entries) * 512(grain) = 1PB
Adding the calculation of vmdk3 to the comment.
In any case, VMware does not offer virtual disks more than 2TB for
vmdk4/vmdk3 or 64TB for the new undocumented seSparse format which is
not implemented yet in qemu.
Reviewed-by: Karl Heubaum
Reviewed-by: Eyal Moscovici
Reviewed-by: Liran Alon
Reviewed-by: Arbel Moshe
Signed-off-by: Sam Eiderman
---
block/vmdk.c | 11 ---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index de8cb859f8..fc7378da78 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -426,10 +426,15 @@ static int vmdk_add_extent(BlockDriverState *bs,
return -EFBIG;
}
if (l1_size > 512 * 1024 * 1024) {
-/* Although with big capacity and small l1_entry_sectors, we
can get a
+/*
+ * Although with big capacity and small l1_entry_sectors, we
can get a
* big l1_size, we don't want unbounded value to allocate the
table.
- * Limit it to 512M, which is 16PB for default cluster and L2
table
- * size */
+ * Limit it to 512M, which is:
+ * 16PB - for default "Hosted Sparse Extent" (VMDK4)
+ *cluster size: 64KB, L2 table size: 512 entries
+ * 1PB - for default "ESXi Host Sparse Extent"
(VMDK3/vmfsSparse)
+ *cluster size: 512B, L2 table size: 4096 entries
+ */
error_setg(errp, "L1 size too big");
return -EFBIG;
}
The calculation of VMDK3 can be verified in end of page No.9 of the spec
(https://www.vmware.com/support/developer/vddk/vmdk_50_technote.pdf).
Also the VMDK4 can be checked in the section Grain Table and Grain in
page No.8 of the spec.
Reviewed-by: yuchenlin
Re: [Qemu-devel] [PATCH] e1000: Delay flush queue when receive RCTL
On 2019-03-25 12:26, Jason Wang wrote:
On 2019/3/21 上午9:35, yuchenlin wrote:
Ping?
On 2019-03-13 14:56, yuchen...@synology.com wrote:
From: yuchenlin
Due to too early RCT0 interrput, win10x32 may hang on booting.
This problem can be reproduced by doing power cycle on win10x32
guest.
In our environment, we have 10 win10x32 and stress power cycle.
The problem will happen about 20 rounds.
Below shows some log with comment:
The normal case:
22831@1551928392.984687:e1000x_rx_disabled Received packet dropped
because receive is disabled RCTL = 0
22831@1551928392.985655:e1000x_rx_disabled Received packet dropped
because receive is disabled RCTL = 0
22831@1551928392.985801:e1000x_rx_disabled Received packet dropped
because receive is disabled RCTL = 0
e1000: set_ics 0, ICR 0, IMR 0
e1000: set_ics 0, ICR 0, IMR 0
e1000: set_ics 0, ICR 0, IMR 0
e1000: RCTL: 0, mac_reg[RCTL] = 0x0
22831@1551928393.056710:e1000x_rx_disabled Received packet dropped
because receive is disabled RCTL = 0
e1000: set_ics 0, ICR 0, IMR 0
e1000: ICR read: 0
e1000: set_ics 0, ICR 0, IMR 0
e1000: set_ics 0, ICR 0, IMR 0
e1000: RCTL: 0, mac_reg[RCTL] = 0x0
22831@1551928393.077548:e1000x_rx_disabled Received packet dropped
because receive is disabled RCTL = 0
e1000: set_ics 0, ICR 0, IMR 0
e1000: ICR read: 0
e1000: set_ics 2, ICR 0, IMR 0
e1000: set_ics 2, ICR 2, IMR 0
e1000: RCTL: 0, mac_reg[RCTL] = 0x0
22831@1551928393.102974:e1000x_rx_disabled Received packet dropped
because receive is disabled RCTL = 0
22831@1551928393.103267:e1000x_rx_disabled Received packet dropped
because receive is disabled RCTL = 0
e1000: RCTL: 255, mac_reg[RCTL] = 0x40002 <- win10x32 says it can
handle
RX now
e1000: set_ics 0, ICR 2, IMR 9d <- unmask interrupt
e1000: RCTL: 255, mac_reg[RCTL] = 0x48002
e1000: set_ics 80, ICR 2, IMR 9d <- interrupt and work!
...
The bad case:
27744@1551930483.117766:e1000x_rx_disabled Received packet dropped
because receive is disabled RCTL = 0
27744@1551930483.118398:e1000x_rx_disabled Received packet dropped
because receive is disabled RCTL = 0
e1000: set_ics 0, ICR 0, IMR 0
e1000: set_ics 0, ICR 0, IMR 0
e1000: set_ics 0, ICR 0, IMR 0
e1000: RCTL: 0, mac_reg[RCTL] = 0x0
27744@1551930483.198063:e1000x_rx_disabled Received packet dropped
because receive is disabled RCTL = 0
e1000: set_ics 0, ICR 0, IMR 0
e1000: ICR read: 0
e1000: set_ics 0, ICR 0, IMR 0
e1000: set_ics 0, ICR 0, IMR 0
e1000: RCTL: 0, mac_reg[RCTL] = 0x0
27744@1551930483.218675:e1000x_rx_disabled Received packet dropped
because receive is disabled RCTL = 0
e1000: set_ics 0, ICR 0, IMR 0
e1000: ICR read: 0
e1000: set_ics 2, ICR 0, IMR 0
e1000: set_ics 2, ICR 2, IMR 0
e1000: RCTL: 0, mac_reg[RCTL] = 0x0
27744@1551930483.241768:e1000x_rx_disabled Received packet dropped
because receive is disabled RCTL = 0
27744@1551930483.241979:e1000x_rx_disabled Received packet dropped
because receive is disabled RCTL = 0
e1000: RCTL: 255, mac_reg[RCTL] = 0x40002 <- win10x32 says it can
handle
RX now
e1000: set_ics 80, ICR 2, IMR 0 <- flush queue (caused by setting
RCTL)
e1000: set_ics 0, ICR 82, IMR 9d <- unmask interrupt and because
0x82&0x9d
!= 0 generate interrupt, hang on here...
Do you mean the interrupt handler is not ready in guest actually?
From my observation, I think yes.
We used to have similar workarounds like autoneg timer, I wonder if we
can reuse that.
IMO, we can't re-use the autoneg timer. The autoneg seems not always be
triggered.
Thanks
Thanks
To workaround this problem, simply delay flush queue. Also stop
receiving
when timer is going to run.
Tested on CentOS, Win7SP1x64 and Win10x32.
Signed-off-by: yuchenlin
---
hw/net/e1000.c | 24 ++--
1 file changed, 22 insertions(+), 2 deletions(-)
diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index 5e144cb4e4..9b39bccfb2 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -120,6 +120,8 @@ typedef struct E1000State_st {
bool mit_irq_level; /* Tracks interrupt pin level. */
uint32_t mit_ide; /* Tracks E1000_TXD_CMD_IDE bit. */
+ QEMUTimer *flush_queue_timer;
+
/* Compatibility flags for migration to/from qemu 1.3.0 and older */
#define E1000_FLAG_AUTONEG_BIT 0
#define E1000_FLAG_MIT_BIT 1
@@ -366,6 +368,7 @@ static void e1000_reset(void *opaque)
timer_del(d->autoneg_timer);
timer_del(d->mit_timer);
+ timer_del(d->flush_queue_timer);
d->mit_timer_on = 0;
d->mit_irq_level = 0;
d->mit_ide = 0;
@@ -391,6 +394,14 @@ set_ctrl(E1000State *s, int index, uint32_t val)
s->mac_reg[CTRL] = val & ~E1000_CTRL_RST;
}
+static void
+e1000_flush_queue_timer(void *opaque)
+{
+ E1000State *s = opaque;
+
+ qemu_flush_queued_packets(qemu_get_queue(s->nic));
+}
+
static void
set_rx_control(E1000State *s, int index, uint32_t val)
{
@@ -399,7 +410,8 @@ set_rx_control(E1000State *s, int index, uint32_t
val)
s->rxbuf_min_shift = ((val / E1000_RCTL_RDMTS_QUAT) & 3) + 1;
DBGOUT(RX, "RCTL: %d, mac_reg[RCTL]
Re: [Qemu-devel] [PATCH] e1000: Delay flush queue when receive RCTL
Ping?
On 2019-03-13 14:56, yuchen...@synology.com wrote:
From: yuchenlin
Due to too early RCT0 interrput, win10x32 may hang on booting.
This problem can be reproduced by doing power cycle on win10x32 guest.
In our environment, we have 10 win10x32 and stress power cycle.
The problem will happen about 20 rounds.
Below shows some log with comment:
The normal case:
22831@1551928392.984687:e1000x_rx_disabled Received packet dropped
because receive is disabled RCTL = 0
22831@1551928392.985655:e1000x_rx_disabled Received packet dropped
because receive is disabled RCTL = 0
22831@1551928392.985801:e1000x_rx_disabled Received packet dropped
because receive is disabled RCTL = 0
e1000: set_ics 0, ICR 0, IMR 0
e1000: set_ics 0, ICR 0, IMR 0
e1000: set_ics 0, ICR 0, IMR 0
e1000: RCTL: 0, mac_reg[RCTL] = 0x0
22831@1551928393.056710:e1000x_rx_disabled Received packet dropped
because receive is disabled RCTL = 0
e1000: set_ics 0, ICR 0, IMR 0
e1000: ICR read: 0
e1000: set_ics 0, ICR 0, IMR 0
e1000: set_ics 0, ICR 0, IMR 0
e1000: RCTL: 0, mac_reg[RCTL] = 0x0
22831@1551928393.077548:e1000x_rx_disabled Received packet dropped
because receive is disabled RCTL = 0
e1000: set_ics 0, ICR 0, IMR 0
e1000: ICR read: 0
e1000: set_ics 2, ICR 0, IMR 0
e1000: set_ics 2, ICR 2, IMR 0
e1000: RCTL: 0, mac_reg[RCTL] = 0x0
22831@1551928393.102974:e1000x_rx_disabled Received packet dropped
because receive is disabled RCTL = 0
22831@1551928393.103267:e1000x_rx_disabled Received packet dropped
because receive is disabled RCTL = 0
e1000: RCTL: 255, mac_reg[RCTL] = 0x40002 <- win10x32 says it can
handle
RX now
e1000: set_ics 0, ICR 2, IMR 9d <- unmask interrupt
e1000: RCTL: 255, mac_reg[RCTL] = 0x48002
e1000: set_ics 80, ICR 2, IMR 9d <- interrupt and work!
...
The bad case:
27744@1551930483.117766:e1000x_rx_disabled Received packet dropped
because receive is disabled RCTL = 0
27744@1551930483.118398:e1000x_rx_disabled Received packet dropped
because receive is disabled RCTL = 0
e1000: set_ics 0, ICR 0, IMR 0
e1000: set_ics 0, ICR 0, IMR 0
e1000: set_ics 0, ICR 0, IMR 0
e1000: RCTL: 0, mac_reg[RCTL] = 0x0
27744@1551930483.198063:e1000x_rx_disabled Received packet dropped
because receive is disabled RCTL = 0
e1000: set_ics 0, ICR 0, IMR 0
e1000: ICR read: 0
e1000: set_ics 0, ICR 0, IMR 0
e1000: set_ics 0, ICR 0, IMR 0
e1000: RCTL: 0, mac_reg[RCTL] = 0x0
27744@1551930483.218675:e1000x_rx_disabled Received packet dropped
because receive is disabled RCTL = 0
e1000: set_ics 0, ICR 0, IMR 0
e1000: ICR read: 0
e1000: set_ics 2, ICR 0, IMR 0
e1000: set_ics 2, ICR 2, IMR 0
e1000: RCTL: 0, mac_reg[RCTL] = 0x0
27744@1551930483.241768:e1000x_rx_disabled Received packet dropped
because receive is disabled RCTL = 0
27744@1551930483.241979:e1000x_rx_disabled Received packet dropped
because receive is disabled RCTL = 0
e1000: RCTL: 255, mac_reg[RCTL] = 0x40002 <- win10x32 says it can
handle
RX now
e1000: set_ics 80, ICR 2, IMR 0 <- flush queue (caused by setting RCTL)
e1000: set_ics 0, ICR 82, IMR 9d <- unmask interrupt and because
0x82&0x9d
!= 0 generate interrupt, hang on here...
To workaround this problem, simply delay flush queue. Also stop
receiving
when timer is going to run.
Tested on CentOS, Win7SP1x64 and Win10x32.
Signed-off-by: yuchenlin
---
hw/net/e1000.c | 24 ++--
1 file changed, 22 insertions(+), 2 deletions(-)
diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index 5e144cb4e4..9b39bccfb2 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -120,6 +120,8 @@ typedef struct E1000State_st {
bool mit_irq_level;/* Tracks interrupt pin level. */
uint32_t mit_ide; /* Tracks E1000_TXD_CMD_IDE bit. */
+QEMUTimer *flush_queue_timer;
+
/* Compatibility flags for migration to/from qemu 1.3.0 and older */
#define E1000_FLAG_AUTONEG_BIT 0
#define E1000_FLAG_MIT_BIT 1
@@ -366,6 +368,7 @@ static void e1000_reset(void *opaque)
timer_del(d->autoneg_timer);
timer_del(d->mit_timer);
+timer_del(d->flush_queue_timer);
d->mit_timer_on = 0;
d->mit_irq_level = 0;
d->mit_ide = 0;
@@ -391,6 +394,14 @@ set_ctrl(E1000State *s, int index, uint32_t val)
s->mac_reg[CTRL] = val & ~E1000_CTRL_RST;
}
+static void
+e1000_flush_queue_timer(void *opaque)
+{
+E1000State *s = opaque;
+
+qemu_flush_queued_packets(qemu_get_queue(s->nic));
+}
+
static void
set_rx_control(E1000State *s, int index, uint32_t val)
{
@@ -399,7 +410,8 @@ set_rx_control(E1000State *s, int index, uint32_t
val)
s->rxbuf_min_shift = ((val / E1000_RCTL_RDMTS_QUAT) & 3) + 1;
DBGOUT(RX, "RCTL: %d, mac_reg[RCTL] = 0x%x\n", s->mac_reg[RDT],
s->mac_reg[RCTL]);
-qemu_flush_queued_packets(qemu_get_queue(s->nic));
+timer_mod(s->flush_queue_timer,
+ qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 1000);
}
static void
@@ -837,7 +849,7 @@ e1000_can_receive(NetClientState *nc)
E1000State *s = qemu_get_nic_opaque(nc);
return e1000x_rx_ready(&s->parent_obj, s->m
[Qemu-devel] [PATCH] e1000: Delay flush queue when receive RCTL
From: yuchenlin
Due to too early RCT0 interrput, win10x32 may hang on booting.
This problem can be reproduced by doing power cycle on win10x32 guest.
In our environment, we have 10 win10x32 and stress power cycle.
The problem will happen about 20 rounds.
Below shows some log with comment:
The normal case:
22831@1551928392.984687:e1000x_rx_disabled Received packet dropped
because receive is disabled RCTL = 0
22831@1551928392.985655:e1000x_rx_disabled Received packet dropped
because receive is disabled RCTL = 0
22831@1551928392.985801:e1000x_rx_disabled Received packet dropped
because receive is disabled RCTL = 0
e1000: set_ics 0, ICR 0, IMR 0
e1000: set_ics 0, ICR 0, IMR 0
e1000: set_ics 0, ICR 0, IMR 0
e1000: RCTL: 0, mac_reg[RCTL] = 0x0
22831@1551928393.056710:e1000x_rx_disabled Received packet dropped
because receive is disabled RCTL = 0
e1000: set_ics 0, ICR 0, IMR 0
e1000: ICR read: 0
e1000: set_ics 0, ICR 0, IMR 0
e1000: set_ics 0, ICR 0, IMR 0
e1000: RCTL: 0, mac_reg[RCTL] = 0x0
22831@1551928393.077548:e1000x_rx_disabled Received packet dropped
because receive is disabled RCTL = 0
e1000: set_ics 0, ICR 0, IMR 0
e1000: ICR read: 0
e1000: set_ics 2, ICR 0, IMR 0
e1000: set_ics 2, ICR 2, IMR 0
e1000: RCTL: 0, mac_reg[RCTL] = 0x0
22831@1551928393.102974:e1000x_rx_disabled Received packet dropped
because receive is disabled RCTL = 0
22831@1551928393.103267:e1000x_rx_disabled Received packet dropped
because receive is disabled RCTL = 0
e1000: RCTL: 255, mac_reg[RCTL] = 0x40002 <- win10x32 says it can handle
RX now
e1000: set_ics 0, ICR 2, IMR 9d <- unmask interrupt
e1000: RCTL: 255, mac_reg[RCTL] = 0x48002
e1000: set_ics 80, ICR 2, IMR 9d <- interrupt and work!
...
The bad case:
27744@1551930483.117766:e1000x_rx_disabled Received packet dropped
because receive is disabled RCTL = 0
27744@1551930483.118398:e1000x_rx_disabled Received packet dropped
because receive is disabled RCTL = 0
e1000: set_ics 0, ICR 0, IMR 0
e1000: set_ics 0, ICR 0, IMR 0
e1000: set_ics 0, ICR 0, IMR 0
e1000: RCTL: 0, mac_reg[RCTL] = 0x0
27744@1551930483.198063:e1000x_rx_disabled Received packet dropped
because receive is disabled RCTL = 0
e1000: set_ics 0, ICR 0, IMR 0
e1000: ICR read: 0
e1000: set_ics 0, ICR 0, IMR 0
e1000: set_ics 0, ICR 0, IMR 0
e1000: RCTL: 0, mac_reg[RCTL] = 0x0
27744@1551930483.218675:e1000x_rx_disabled Received packet dropped
because receive is disabled RCTL = 0
e1000: set_ics 0, ICR 0, IMR 0
e1000: ICR read: 0
e1000: set_ics 2, ICR 0, IMR 0
e1000: set_ics 2, ICR 2, IMR 0
e1000: RCTL: 0, mac_reg[RCTL] = 0x0
27744@1551930483.241768:e1000x_rx_disabled Received packet dropped
because receive is disabled RCTL = 0
27744@1551930483.241979:e1000x_rx_disabled Received packet dropped
because receive is disabled RCTL = 0
e1000: RCTL: 255, mac_reg[RCTL] = 0x40002 <- win10x32 says it can handle
RX now
e1000: set_ics 80, ICR 2, IMR 0 <- flush queue (caused by setting RCTL)
e1000: set_ics 0, ICR 82, IMR 9d <- unmask interrupt and because 0x82&0x9d
!= 0 generate interrupt, hang on here...
To workaround this problem, simply delay flush queue. Also stop receiving
when timer is going to run.
Tested on CentOS, Win7SP1x64 and Win10x32.
Signed-off-by: yuchenlin
---
hw/net/e1000.c | 24 ++--
1 file changed, 22 insertions(+), 2 deletions(-)
diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index 5e144cb4e4..9b39bccfb2 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -120,6 +120,8 @@ typedef struct E1000State_st {
bool mit_irq_level;/* Tracks interrupt pin level. */
uint32_t mit_ide; /* Tracks E1000_TXD_CMD_IDE bit. */
+QEMUTimer *flush_queue_timer;
+
/* Compatibility flags for migration to/from qemu 1.3.0 and older */
#define E1000_FLAG_AUTONEG_BIT 0
#define E1000_FLAG_MIT_BIT 1
@@ -366,6 +368,7 @@ static void e1000_reset(void *opaque)
timer_del(d->autoneg_timer);
timer_del(d->mit_timer);
+timer_del(d->flush_queue_timer);
d->mit_timer_on = 0;
d->mit_irq_level = 0;
d->mit_ide = 0;
@@ -391,6 +394,14 @@ set_ctrl(E1000State *s, int index, uint32_t val)
s->mac_reg[CTRL] = val & ~E1000_CTRL_RST;
}
+static void
+e1000_flush_queue_timer(void *opaque)
+{
+E1000State *s = opaque;
+
+qemu_flush_queued_packets(qemu_get_queue(s->nic));
+}
+
static void
set_rx_control(E1000State *s, int index, uint32_t val)
{
@@ -399,7 +410,8 @@ set_rx_control(E1000State *s, int index, uint32_t val)
s->rxbuf_min_shift = ((val / E1000_RCTL_RDMTS_QUAT) & 3) + 1;
DBGOUT(RX, "RCTL: %d, mac_reg[RCTL] = 0x%x\n", s->mac_reg[RDT],
s->mac_reg[RCTL]);
-qemu_flush_queued_packets(qemu_get_queue(s->nic));
+timer_mod(s->flush_queue_timer,
+ qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 1000);
}
static void
@@ -837,7 +849,7 @@ e1000_can_receive(NetClientState *nc)
E1000State *s = qemu_get_nic_opaque(nc);
return e1000x_rx_ready(&s->parent_obj, s->mac_reg) &&
-e1000_has_rxbufs(s, 1);
+e1000_has_rx
[Qemu-devel] [PATCH] vmdk: false positive of compat6 with hwversion not set
From: yuchenlin
In vmdk_co_create_opts, when it finds hw_version is undefined, it will
set it to 4, which misleading the compat6 and hwversion in
vmdk_co_do_create. Simply set hw_version to NULL after free, let
the logic in vmdk_co_do_create to decide the value of hw_version.
This bug can be reproduced by:
$ qemu-img convert -O vmdk -o subformat=streamOptimized,compat6
/home/yuchenlin/syno.qcow2 /home/yuchenlin/syno.vmdk
qemu-img: /home/yuchenlin/syno.vmdk: error while converting vmdk:
compat6 cannot be enabled with hwversion set
Signed-off-by: yuchenlin
---
block/vmdk.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index 096e8eb662..e3bbd18803 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -2260,7 +2260,7 @@ static int coroutine_fn vmdk_co_create_opts(const char
*filename, QemuOpts *opts
compat6 = qemu_opt_get_bool_del(opts, BLOCK_OPT_COMPAT6, false);
if (strcmp(hw_version, "undefined") == 0) {
g_free(hw_version);
-hw_version = g_strdup("4");
+hw_version = NULL;
}
fmt = qemu_opt_get_del(opts, BLOCK_OPT_SUBFMT);
zeroed_grain = qemu_opt_get_bool_del(opts, BLOCK_OPT_ZEROED_GRAIN, false);
--
2.17.1
Re: [Qemu-devel] [Qemu-block] [PATCH] dmg: Fixing wrong dmg block type value for block terminator.
On 2018-12-28 22:50, Julio Faracco wrote:
This is a trivial patch to fix a wrong value for block terminator.
The old value was 0x7fff which is wrong. It was not affecting the
code because QEMU dmg block is not handling block terminator right now.
Neverthless, it should be fixed.
Signed-off-by: Julio Faracco
Reviewed-by: yuchenlin
---
block/dmg.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/block/dmg.c b/block/dmg.c
index 50e91aef6d..2c806e3389 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -54,7 +54,7 @@ enum {
UDBZ,
ULFO,
UDCM = 0x7ffe, /* Comments */
-UDLE /* Last Entry */
+UDLE = 0x /* Last Entry */
};
static int dmg_probe(const uint8_t *buf, int buf_size, const char
*filename)
Re: [Qemu-devel] [PATCH] vga_int: remove unused function protype
On 2018-10-29 17:44, Gerd Hoffmann wrote: On Mon, Oct 22, 2018 at 04:00:53PM +0800, yuchen...@synology.com wrote: From: yuchenlin Signed-off-by: yuchenlin --- hw/display/vga_int.h | 1 - 1 file changed, 1 deletion(-) diff --git a/hw/display/vga_int.h b/hw/display/vga_int.h index 6e4fa48a79..55c418eab5 100644 --- a/hw/display/vga_int.h +++ b/hw/display/vga_int.h @@ -166,7 +166,6 @@ MemoryRegion *vga_init_io(VGACommonState *s, Object *obj, const MemoryRegionPortio **vbe_ports); void vga_common_reset(VGACommonState *s); -void vga_sync_dirty_bitmap(VGACommonState *s); void vga_dirty_log_start(VGACommonState *s); void vga_dirty_log_stop(VGACommonState *s); Added to vga queue. thanks, Gerd Hi, Gerd Laurent has sent a pull request for this trivial commit. See: http://lists.nongnu.org/archive/html/qemu-devel/2018-10/msg05896.html Thanks, yuchenlin
[Qemu-devel] [PATCH] vga_int: remove unused function protype
From: yuchenlin Signed-off-by: yuchenlin --- hw/display/vga_int.h | 1 - 1 file changed, 1 deletion(-) diff --git a/hw/display/vga_int.h b/hw/display/vga_int.h index 6e4fa48a79..55c418eab5 100644 --- a/hw/display/vga_int.h +++ b/hw/display/vga_int.h @@ -166,7 +166,6 @@ MemoryRegion *vga_init_io(VGACommonState *s, Object *obj, const MemoryRegionPortio **vbe_ports); void vga_common_reset(VGACommonState *s); -void vga_sync_dirty_bitmap(VGACommonState *s); void vga_dirty_log_start(VGACommonState *s); void vga_dirty_log_stop(VGACommonState *s); -- 2.18.0
Re: [Qemu-devel] [PATCH] vhost-scsi: prevent using uninitialized vqs
Ping? On 2018-10-12 17:07, yuchen...@synology.com wrote: From: yuchenlin There are 3 virtqueues (ctrl, event and cmd) for virtio scsi device, but seabios will only set the physical address for the 3rd one (cmd). Then in vhost_virtqueue_start(), virtio_queue_get_desc_addr() will be 0 for ctrl and event vq. In this case, ctrl and event vq are not initialized. vhost_verify_ring_mappings may use uninitialized vhost_virtqueue such that vhost_verify_ring_part_mapping returns ENOMEM. When encountered this problem, we got the following logs: qemu-system-x86_64: Unable to map available ring for ring 0 qemu-system-x86_64: Verify ring failure on region 0 Signed-off-by: Forrest Liu Signed-off-by: yuchenlin --- hw/scsi/vhost-scsi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c index becf550085..7f21b4f9d6 100644 --- a/hw/scsi/vhost-scsi.c +++ b/hw/scsi/vhost-scsi.c @@ -183,7 +183,7 @@ static void vhost_scsi_realize(DeviceState *dev, Error **errp) } vsc->dev.nvqs = VHOST_SCSI_VQ_NUM_FIXED + vs->conf.num_queues; -vsc->dev.vqs = g_new(struct vhost_virtqueue, vsc->dev.nvqs); +vsc->dev.vqs = g_new0(struct vhost_virtqueue, vsc->dev.nvqs); vsc->dev.vq_index = 0; vsc->dev.backend_features = 0;
[Qemu-devel] [PATCH] vhost-scsi: prevent using uninitialized vqs
From: yuchenlin There are 3 virtqueues (ctrl, event and cmd) for virtio scsi device, but seabios will only set the physical address for the 3rd one (cmd). Then in vhost_virtqueue_start(), virtio_queue_get_desc_addr() will be 0 for ctrl and event vq. In this case, ctrl and event vq are not initialized. vhost_verify_ring_mappings may use uninitialized vhost_virtqueue such that vhost_verify_ring_part_mapping returns ENOMEM. When encountered this problem, we got the following logs: qemu-system-x86_64: Unable to map available ring for ring 0 qemu-system-x86_64: Verify ring failure on region 0 Signed-off-by: Forrest Liu Signed-off-by: yuchenlin --- hw/scsi/vhost-scsi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c index becf550085..7f21b4f9d6 100644 --- a/hw/scsi/vhost-scsi.c +++ b/hw/scsi/vhost-scsi.c @@ -183,7 +183,7 @@ static void vhost_scsi_realize(DeviceState *dev, Error **errp) } vsc->dev.nvqs = VHOST_SCSI_VQ_NUM_FIXED + vs->conf.num_queues; -vsc->dev.vqs = g_new(struct vhost_virtqueue, vsc->dev.nvqs); +vsc->dev.vqs = g_new0(struct vhost_virtqueue, vsc->dev.nvqs); vsc->dev.vq_index = 0; vsc->dev.backend_features = 0; -- 2.18.0
Re: [Qemu-devel] [PATCH v3] vmdk: align end of file to a sector boundary
On 2018-10-08 10:38, Fam Zheng wrote: On Fri, 10/05 10:00, yuchenlin wrote: Ping? Hi, This was merged as 51b3c6b73acae1e3fd3c7d441fc86dd17356695f. Fam Hi, Thank you for your information. yuchenlin On 2018-09-13 16:34, Fam Zheng wrote: > On Thu, 09/13 16:29, yuchen...@synology.com wrote: > > From: yuchenlin > > > > There is a rare case which the size of last compressed cluster > > is larger than the cluster size, which will cause the file is > > not aligned at the sector boundary. > > > > There are three reasons to do it. First, if vmdk doesn't align at > > the sector boundary, there may be many undefined behaviors, > > such as, in vbox it will show VMDK: Compressed image is corrupted > > 'syno-vm-disk1.vmdk' (VERR_ZIP_CORRUPTED) when we try to import an > > ova with unaligned vmdk. Second, all the cluster_sector is aligned > > to sector, the last one should be like this, too. Third, it ease > > reading with sector based I/Os. > > > > Signed-off-by: yuchenlin > > Reviewed-by: Fam Zheng
Re: [Qemu-devel] [PATCH v3] vmdk: align end of file to a sector boundary
Ping? On 2018-09-13 16:34, Fam Zheng wrote: On Thu, 09/13 16:29, yuchen...@synology.com wrote: From: yuchenlin There is a rare case which the size of last compressed cluster is larger than the cluster size, which will cause the file is not aligned at the sector boundary. There are three reasons to do it. First, if vmdk doesn't align at the sector boundary, there may be many undefined behaviors, such as, in vbox it will show VMDK: Compressed image is corrupted 'syno-vm-disk1.vmdk' (VERR_ZIP_CORRUPTED) when we try to import an ova with unaligned vmdk. Second, all the cluster_sector is aligned to sector, the last one should be like this, too. Third, it ease reading with sector based I/Os. Signed-off-by: yuchenlin Reviewed-by: Fam Zheng
[Qemu-devel] [PATCH v3] vmdk: align end of file to a sector boundary
From: yuchenlin
There is a rare case which the size of last compressed cluster
is larger than the cluster size, which will cause the file is
not aligned at the sector boundary.
There are three reasons to do it. First, if vmdk doesn't align at
the sector boundary, there may be many undefined behaviors,
such as, in vbox it will show VMDK: Compressed image is corrupted
'syno-vm-disk1.vmdk' (VERR_ZIP_CORRUPTED) when we try to import an
ova with unaligned vmdk. Second, all the cluster_sector is aligned
to sector, the last one should be like this, too. Third, it ease
reading with sector based I/Os.
Signed-off-by: yuchenlin
---
v2 -> v3:
* Update commit message
thanks
block/vmdk.c | 21 +
1 file changed, 21 insertions(+)
diff --git a/block/vmdk.c b/block/vmdk.c
index a9d0084e36..2c9e86d98f 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1698,6 +1698,27 @@ static int coroutine_fn
vmdk_co_pwritev_compressed(BlockDriverState *bs, uint64_t offset,
uint64_t bytes, QEMUIOVector *qiov)
{
+if (bytes == 0) {
+/* The caller will write bytes 0 to signal EOF.
+ * When receive it, we align EOF to a sector boundary. */
+BDRVVmdkState *s = bs->opaque;
+int i, ret;
+int64_t length;
+
+for (i = 0; i < s->num_extents; i++) {
+length = bdrv_getlength(s->extents[i].file->bs);
+if (length < 0) {
+return length;
+}
+length = QEMU_ALIGN_UP(length, BDRV_SECTOR_SIZE);
+ret = bdrv_truncate(s->extents[i].file, length,
+PREALLOC_MODE_OFF, NULL);
+if (ret < 0) {
+return ret;
+}
+}
+return 0;
+}
return vmdk_co_pwritev(bs, offset, bytes, qiov, 0);
}
--
2.18.0
Re: [Qemu-devel] [PATCH v2] vmdk: align end of file to a sector boundary
On 2018-09-13 10:54, Fam Zheng wrote:
On Thu, 09/13 10:31, yuchen...@synology.com wrote:
From: yuchenlin
There is a rare case which the size of last compressed cluster
is larger than the cluster size, which will cause the file is
not aligned at the sector boundary.
The code looks good to me. Can you also explain why it is important to
align
file size to sector boundary in the comment?
Fam
In my opinion, there are three reasons to do it. First, if vmdk doesn't
align at the sector boundary, there may be many undefined behaviors,
such as in vbox it will show VMDK: Compressed image is corrupted
'88-disk1.vmdk' (VERR_ZIP_CORRUPTED) when we try to import an ova with
unaligned vmdk. Second, all the cluster_sector is aligned
to sector, the last one should be like this, too. Third, it ease reading
with sector based I/Os.
What do you think?
yuchenlin
Signed-off-by: yuchenlin
---
v1 -> v2:
* Add more detail comment.
* Add QEMU_ALIGN_UP to show the intention more clearly.
* Add newline in the end of bdrv_truncate.
thanks
block/vmdk.c | 21 +
1 file changed, 21 insertions(+)
diff --git a/block/vmdk.c b/block/vmdk.c
index a9d0084e36..2c9e86d98f 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1698,6 +1698,27 @@ static int coroutine_fn
vmdk_co_pwritev_compressed(BlockDriverState *bs, uint64_t offset,
uint64_t bytes, QEMUIOVector *qiov)
{
+if (bytes == 0) {
+/* The caller will write bytes 0 to signal EOF.
+ * When receive it, we align EOF to a sector boundary. */
+BDRVVmdkState *s = bs->opaque;
+int i, ret;
+int64_t length;
+
+for (i = 0; i < s->num_extents; i++) {
+length = bdrv_getlength(s->extents[i].file->bs);
+if (length < 0) {
+return length;
+}
+length = QEMU_ALIGN_UP(length, BDRV_SECTOR_SIZE);
+ret = bdrv_truncate(s->extents[i].file, length,
+PREALLOC_MODE_OFF, NULL);
+if (ret < 0) {
+return ret;
+}
+}
+return 0;
+}
return vmdk_co_pwritev(bs, offset, bytes, qiov, 0);
}
--
2.18.0
[Qemu-devel] [PATCH v2] vmdk: align end of file to a sector boundary
From: yuchenlin
There is a rare case which the size of last compressed cluster
is larger than the cluster size, which will cause the file is
not aligned at the sector boundary.
Signed-off-by: yuchenlin
---
v1 -> v2:
* Add more detail comment.
* Add QEMU_ALIGN_UP to show the intention more clearly.
* Add newline in the end of bdrv_truncate.
thanks
block/vmdk.c | 21 +
1 file changed, 21 insertions(+)
diff --git a/block/vmdk.c b/block/vmdk.c
index a9d0084e36..2c9e86d98f 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1698,6 +1698,27 @@ static int coroutine_fn
vmdk_co_pwritev_compressed(BlockDriverState *bs, uint64_t offset,
uint64_t bytes, QEMUIOVector *qiov)
{
+if (bytes == 0) {
+/* The caller will write bytes 0 to signal EOF.
+ * When receive it, we align EOF to a sector boundary. */
+BDRVVmdkState *s = bs->opaque;
+int i, ret;
+int64_t length;
+
+for (i = 0; i < s->num_extents; i++) {
+length = bdrv_getlength(s->extents[i].file->bs);
+if (length < 0) {
+return length;
+}
+length = QEMU_ALIGN_UP(length, BDRV_SECTOR_SIZE);
+ret = bdrv_truncate(s->extents[i].file, length,
+PREALLOC_MODE_OFF, NULL);
+if (ret < 0) {
+return ret;
+}
+}
+return 0;
+}
return vmdk_co_pwritev(bs, offset, bytes, qiov, 0);
}
--
2.18.0
Re: [Qemu-devel] [PATCH] vmdk: align end of file to a sector boundary
On 2018-09-12 19:54, Fam Zheng wrote:
On Tue, 08/28 11:17, yuchen...@synology.com wrote:
From: yuchenlin
There is a rare case which the size of last compressed cluster
is larger than the cluster size, which will cause the file is
not aligned at the sector boundary.
Signed-off-by: yuchenlin
---
block/vmdk.c | 18 ++
1 file changed, 18 insertions(+)
diff --git a/block/vmdk.c b/block/vmdk.c
index a9d0084e36..a8ae7c65d2 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1698,6 +1698,24 @@ static int coroutine_fn
vmdk_co_pwritev_compressed(BlockDriverState *bs, uint64_t offset,
uint64_t bytes, QEMUIOVector *qiov)
{
+if (bytes == 0) {
+/* align end of file to a sector boundary. */
+BDRVVmdkState *s = bs->opaque;
+int i, ret;
+int64_t length;
+
+for (i = 0; i < s->num_extents; i++) {
+length = bdrv_getlength(s->extents[i].file->bs);
+if (length < 0) {
+return length;
+}
Could you add "length = QEMU_ALIGN_UP(length, BDRV_SECTOR_SIZE);" to
show the
intention more clearly?
Fam
Thank you for your effort, I will do it.
yuchenlin
+ret = bdrv_truncate(s->extents[i].file, length,
PREALLOC_MODE_OFF, NULL);
+if (ret < 0) {
+return ret;
+}
+}
+return 0;
+}
return vmdk_co_pwritev(bs, offset, bytes, qiov, 0);
}
--
2.17.0
Re: [Qemu-devel] [PATCH] vmdk: align end of file to a sector boundary
Fam Zheng 於 2018-09-12 17:34 寫道：
> On Tue, 08/28 11:17, yuchen...@synology.com wrote: > From: yuchenlin
> > > There is a rare case which the size of last
> compressed cluster > is larger than the cluster size, which will cause the
> file is > not aligned at the sector boundary. I don't understand. Doesn't it
> mean that if you force the alignment by truncating out the extra bytes, some
> data is lost?
You can take qcow2_co_pwritev_compressed in block/qcow2.c as an example.
The bdrv_getlength will return the length in bytes which is always a multiple
of BDRV_SECTOR_SIZE.
After truncates this size, the vmdk is extended to align sector size.
> > > Signed-off-by: yuchenlin > --- > block/vmdk.c |
> > > 18 ++ > 1 file changed, 18 insertions(+) > > diff --git
> > > a/block/vmdk.c b/block/vmdk.c > index a9d0084e36..a8ae7c65d2 100644 > ---
> > > a/block/vmdk.c > +++ b/block/vmdk.c > @@ -1698,6 +1698,24 @@ static int
> > > coroutine_fn > vmdk_co_pwritev_compressed(BlockDriverState *bs, uint64_t
> > > offset, > uint64_t bytes, QEMUIOVector *qiov) > { > + if (bytes == 0) {
> > > Where is this bytes == 0 condition from?
From the end of convert_do_copy in qemu-img.c.
if (s->compressed && !s->ret) {
/* signal EOF to align */
ret = blk_pwrite_compressed(s->target, 0, NULL, 0);
if (ret < 0) {
return ret;
}
}
It signals the EOF to the block driver.
> > + /* align end of file to a sector boundary. */ > + BDRVVmdkState *s =
> > bs->opaque; > + int i, ret; > + int64_t length; > + > + for (i = 0; i <
> > s->num_extents; i++) { > + length = bdrv_getlength(s->extents[i].file->bs);
> > > + if (length < 0) { > + return length; > + } > + ret =
> > bdrv_truncate(s->extents[i].file, length, PREALLOC_MODE_OFF, NULL); > + if
> > (ret < 0) { > + return ret; > + } > + } > + return 0; > + } > return
> > vmdk_co_pwritev(bs, offset, bytes, qiov, 0); > } > > -- > 2.17.0 > Fam
yuchenlin
Re: [Qemu-devel] [PATCH] vmdk: align end of file to a sector boundary
Ping!
yuchen...@synology.com 於 2018-08-28 11:18 寫道：
> From: yuchenlin There is a rare case which the size
> of last compressed cluster is larger than the cluster size, which will cause
> the file is not aligned at the sector boundary. Signed-off-by: yuchenlin
> --- block/vmdk.c | 18 ++ 1 file
> changed, 18 insertions(+) diff --git a/block/vmdk.c b/block/vmdk.c index
> a9d0084e36..a8ae7c65d2 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@
> -1698,6 +1698,24 @@ static int coroutine_fn
> vmdk_co_pwritev_compressed(BlockDriverState *bs, uint64_t offset, uint64_t
> bytes, QEMUIOVector *qiov) { + if (bytes == 0) { + /* align end of file to a
> sector boundary. */ + BDRVVmdkState *s = bs->opaque; + int i, ret; + int64_t
> length; + + for (i = 0; i < s->num_extents; i++) { + length =
> bdrv_getlength(s->extents[i].file->bs); + if (length < 0) { + return length;
> + } + ret = bdrv_truncate(s->extents[i].file, length, PREALLOC_MODE_OFF,
> NULL); + if (ret < 0) { + return ret; + } + } + return 0; + } return
> vmdk_co_pwritev(bs, offset, bytes, qiov, 0); } -- 2.17.0
[Qemu-devel] [PATCH] vmdk: align end of file to a sector boundary
From: yuchenlin
There is a rare case which the size of last compressed cluster
is larger than the cluster size, which will cause the file is
not aligned at the sector boundary.
Signed-off-by: yuchenlin
---
block/vmdk.c | 18 ++
1 file changed, 18 insertions(+)
diff --git a/block/vmdk.c b/block/vmdk.c
index a9d0084e36..a8ae7c65d2 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1698,6 +1698,24 @@ static int coroutine_fn
vmdk_co_pwritev_compressed(BlockDriverState *bs, uint64_t offset,
uint64_t bytes, QEMUIOVector *qiov)
{
+if (bytes == 0) {
+/* align end of file to a sector boundary. */
+BDRVVmdkState *s = bs->opaque;
+int i, ret;
+int64_t length;
+
+for (i = 0; i < s->num_extents; i++) {
+length = bdrv_getlength(s->extents[i].file->bs);
+if (length < 0) {
+return length;
+}
+ret = bdrv_truncate(s->extents[i].file, length, PREALLOC_MODE_OFF,
NULL);
+if (ret < 0) {
+return ret;
+}
+}
+return 0;
+}
return vmdk_co_pwritev(bs, offset, bytes, qiov, 0);
}
--
2.17.0
Re: [Qemu-devel] [PATCH 0/2] Refine some vdi code
Hi, Stefan I agree that redundancy of If else may helps people to understand the code. However, CONFIG_VDI_WRITE only contributes: #if defined(CONFIG_VDI_WRITE) .bdrv_co_pwritev = vdi_co_pwritev, #endif I think we don't need CONFIG_VDI_WRITE to document the code. As its name implies, vdi_co_pwritev shows the code parts for vdi write support. I appreciated your time and effort for reviews. Regards, yuchenlin Stefan Weil 於 2018-07-30 15:13 寫道： > Am 30.07.2018 um 04:46 schrieb yuchen...@synology.com: > From: yuchenlin > > > This series refine some code in vdi.c, includes: > > > * Remvoe CONFIG_VDI_WRITE because there is no reason to leave an always > on > and cannot configure option in the code-side. > * decouple if else if > chain to get more readability. > > Thanks, > yuchenlin > > yuchenlin (2): > > vdi: remove CONFIG_VDI_WRITE > vdi: refine code for vdi_open > > block/vdi.c > | 32 ++-- > 1 file changed, 18 insertions(+), 14 > deletions(-) Technically these changes are fine, but personally I prefer my > old code. If else is rendundant here, but redundancy helps humans to > understand the code. CONFIG_VDI_WRITE still has a similar function as it > documents which code parts are relevant for write support. Stefan
[Qemu-devel] [PATCH 1/2] vdi: remove CONFIG_VDI_WRITE
From: yuchenlin
The CONFIG_VDI_WRITE is here when the first time vdi is added.
But there is no reason to leave an always on and cannot configure option
in the code-side.
Signed-off-by: yuchenlin
---
block/vdi.c | 5 -
1 file changed, 5 deletions(-)
diff --git a/block/vdi.c b/block/vdi.c
index 6555cffb88..12f92e7891 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -70,9 +70,6 @@
/* Enable debug messages. */
//~ #define CONFIG_VDI_DEBUG
-/* Support write operations on VDI images. */
-#define CONFIG_VDI_WRITE
-
/* Support non-standard block (cluster) size. This is untested.
* Maybe it will be needed for very large images.
*/
@@ -1016,9 +1013,7 @@ static BlockDriver bdrv_vdi = {
.bdrv_make_empty = vdi_make_empty,
.bdrv_co_preadv = vdi_co_preadv,
-#if defined(CONFIG_VDI_WRITE)
.bdrv_co_pwritev= vdi_co_pwritev,
-#endif
.bdrv_get_info = vdi_get_info,
--
2.17.0
[Qemu-devel] [PATCH 0/2] Refine some vdi code
From: yuchenlin This series refine some code in vdi.c, includes: * Remvoe CONFIG_VDI_WRITE because there is no reason to leave an always on and cannot configure option in the code-side. * decouple if else if chain to get more readability. Thanks, yuchenlin yuchenlin (2): vdi: remove CONFIG_VDI_WRITE vdi: refine code for vdi_open block/vdi.c | 32 ++-- 1 file changed, 18 insertions(+), 14 deletions(-) -- 2.17.0
[Qemu-devel] [PATCH 2/2] vdi: refine code for vdi_open
From: yuchenlin
When the condition of each if or else if is true,
the code flow will goto fail. Which means we can decouple
if else if chain to get some readability.
Signed-off-by: yuchenlin
---
block/vdi.c | 27 ++-
1 file changed, 18 insertions(+), 9 deletions(-)
diff --git a/block/vdi.c b/block/vdi.c
index 12f92e7891..28fc6210a7 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -405,35 +405,41 @@ static int vdi_open(BlockDriverState *bs, QDict *options,
int flags,
")", header.signature);
ret = -EINVAL;
goto fail;
-} else if (header.version != VDI_VERSION_1_1) {
+}
+if (header.version != VDI_VERSION_1_1) {
error_setg(errp, "unsupported VDI image (version %" PRIu32 ".%" PRIu32
")", header.version >> 16, header.version & 0x);
ret = -ENOTSUP;
goto fail;
-} else if (header.offset_bmap % SECTOR_SIZE != 0) {
+}
+if (header.offset_bmap % SECTOR_SIZE != 0) {
/* We only support block maps which start on a sector boundary. */
error_setg(errp, "unsupported VDI image (unaligned block map offset "
"0x%" PRIx32 ")", header.offset_bmap);
ret = -ENOTSUP;
goto fail;
-} else if (header.offset_data % SECTOR_SIZE != 0) {
+}
+if (header.offset_data % SECTOR_SIZE != 0) {
/* We only support data blocks which start on a sector boundary. */
error_setg(errp, "unsupported VDI image (unaligned data offset 0x%"
PRIx32 ")", header.offset_data);
ret = -ENOTSUP;
goto fail;
-} else if (header.sector_size != SECTOR_SIZE) {
+}
+if (header.sector_size != SECTOR_SIZE) {
error_setg(errp, "unsupported VDI image (sector size %" PRIu32
" is not %u)", header.sector_size, SECTOR_SIZE);
ret = -ENOTSUP;
goto fail;
-} else if (header.block_size != DEFAULT_CLUSTER_SIZE) {
+}
+if (header.block_size != DEFAULT_CLUSTER_SIZE) {
error_setg(errp, "unsupported VDI image (block size %" PRIu32
" is not %" PRIu64 ")",
header.block_size, DEFAULT_CLUSTER_SIZE);
ret = -ENOTSUP;
goto fail;
-} else if (header.disk_size >
+}
+if (header.disk_size >
(uint64_t)header.blocks_in_image * header.block_size) {
error_setg(errp, "unsupported VDI image (disk size %" PRIu64 ", "
"image bitmap has room for %" PRIu64 ")",
@@ -441,15 +447,18 @@ static int vdi_open(BlockDriverState *bs, QDict *options,
int flags,
(uint64_t)header.blocks_in_image * header.block_size);
ret = -ENOTSUP;
goto fail;
-} else if (!qemu_uuid_is_null(&header.uuid_link)) {
+}
+if (!qemu_uuid_is_null(&header.uuid_link)) {
error_setg(errp, "unsupported VDI image (non-NULL link UUID)");
ret = -ENOTSUP;
goto fail;
-} else if (!qemu_uuid_is_null(&header.uuid_parent)) {
+}
+if (!qemu_uuid_is_null(&header.uuid_parent)) {
error_setg(errp, "unsupported VDI image (non-NULL parent UUID)");
ret = -ENOTSUP;
goto fail;
-} else if (header.blocks_in_image > VDI_BLOCKS_IN_IMAGE_MAX) {
+}
+if (header.blocks_in_image > VDI_BLOCKS_IN_IMAGE_MAX) {
error_setg(errp, "unsupported VDI image "
"(too many blocks %u, max is %u)",
header.blocks_in_image, VDI_BLOCKS_IN_IMAGE_MAX);
--
2.17.0
[Qemu-devel] [PATCH v3] vmdk: return ERROR when cluster sector is larger than vmdk limitation
From: yuchenlin
VMDK has a hard limitation of extent size, which is due to the size of grain
table entry is 32 bits. It means it can only point to a grain located at
offset = 2^32. To avoid writing the user data beyond limitation and record a
useless offset
in grain table. We should return ERROR here.
Signed-off-by: yuchenlin
---
v2->v3:
- use (1ULL << 32) to clearly show the limitation of offset is 2^32.
thanks
block/vmdk.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/block/vmdk.c b/block/vmdk.c
index f94c49a9c0..84f8bbe480 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -47,6 +47,8 @@
#define VMDK4_FLAG_MARKER (1 << 17)
#define VMDK4_GD_AT_END 0xULL
+#define VMDK_EXTENT_MAX_SECTORS (1ULL << 32)
+
#define VMDK_GTE_ZEROED 0x1
/* VMDK internal error codes */
@@ -1250,6 +1252,10 @@ static int get_cluster_offset(BlockDriverState *bs,
return zeroed ? VMDK_ZEROED : VMDK_UNALLOC;
}
+if (extent->next_cluster_sector >= VMDK_EXTENT_MAX_SECTORS) {
+return VMDK_ERROR;
+}
+
cluster_sector = extent->next_cluster_sector;
extent->next_cluster_sector += extent->cluster_sectors;
--
2.16.2
[Qemu-devel] [PATCH v2] vmdk: return ERROR when cluster sector is larger than vmdk limitation
From: yuchenlin
VMDK has a hard limitation of extent size, which is due to the size of grain
table entry is 32 bits. It means it can only point to a grain located at
offset = 2^32. To avoid writing the user data beyond limitation and record a
useless offset
in grain table. We should return ERROR here.
Signed-off-by: yuchenlin
---
v1->v2:
- change commit message
- check before allocating
- should be >=
- the unit is sector now
thanks
block/vmdk.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/block/vmdk.c b/block/vmdk.c
index f94c49a9c0..a1c21dbbba 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -47,6 +47,8 @@
#define VMDK4_FLAG_MARKER (1 << 17)
#define VMDK4_GD_AT_END 0xULL
+#define VMDK_EXTENT_MAX_SECTORS (4294967296)
+
#define VMDK_GTE_ZEROED 0x1
/* VMDK internal error codes */
@@ -1250,6 +1252,10 @@ static int get_cluster_offset(BlockDriverState *bs,
return zeroed ? VMDK_ZEROED : VMDK_UNALLOC;
}
+if (extent->next_cluster_sector >= VMDK_EXTENT_MAX_SECTORS) {
+return VMDK_ERROR;
+}
+
cluster_sector = extent->next_cluster_sector;
extent->next_cluster_sector += extent->cluster_sectors;
--
2.16.2
[Qemu-devel] [PATCH] vmdk: return ENOTSUP before offset overflow
From: yuchenlin
VMDK has a hard limitation of extent size, which is due to the size of grain
table entry is 32 bits. It means it can only point to a grain located at
offset = 2^32. To prevent offset overflow and record a useless offset
in grain table. We should return un-support here.
Signed-off-by: yuchenlin
---
block/vmdk.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/block/vmdk.c b/block/vmdk.c
index f94c49a9c0..d8fc961940 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -47,6 +47,9 @@
#define VMDK4_FLAG_MARKER (1 << 17)
#define VMDK4_GD_AT_END 0xULL
+/* 2TB */
+#define VMDK_EXTENT_SIZE_LIMIT (219902322)
+
#define VMDK_GTE_ZEROED 0x1
/* VMDK internal error codes */
@@ -1645,6 +1648,9 @@ static int vmdk_pwritev(BlockDriverState *bs, uint64_t
offset,
return ret;
}
if (m_data.valid) {
+if (cluster_offset > VMDK_EXTENT_SIZE_LIMIT) {
+return -ENOTSUP;
+}
/* update L2 tables */
if (vmdk_L2update(extent, &m_data,
cluster_offset >> BDRV_SECTOR_BITS)
--
2.16.2
