Re: [Qemu-devel] [PULL 00/24] target/hppa patch queue
On Tue, 12 Feb 2019 at 04:57, Richard Henderson wrote: > > The following changes since commit 22c5f446514a2a4bb0dbe1fea26713da92fc85fa: > > Merge remote-tracking branch 'remotes/rth/tags/pull-tcg-20190211' into > staging (2019-02-11 17:04:57 +) > > are available in the Git repository at: > > https://github.com/rth7680/qemu.git tags/pull-hppa-20190211 > > for you to fetch changes up to 23e76627deba013509b5f1a1e1c53e8e45aa: > > hw/hppa: forward requests to CPU HPA (2019-02-11 20:49:06 -0800) > > > Convert to decodetree. > Fix signed overflow conditions. > Fix dcor. > Add CPU MIE to PCI address space. Hi -- clang compiles fail with: In file included from /home/petmay01/linaro/qemu-for-merges/target/hppa/translate.c:337: target/hppa/decode.inc.c:471:16: error: redefinition of typedef 'arg_be' is a C11 feature [-Werror,-Wtypedef-redefinition] typedef arg_be arg_be; ^ target/hppa/decode.inc.c:9:3: note: previous definition is here } arg_be; ^ target/hppa/decode.inc.c:473:16: error: redefinition of typedef 'arg_bl' is a C11 feature [-Werror,-Wtypedef-redefinition] typedef arg_bl arg_bl; ^ target/hppa/decode.inc.c:15:3: note: previous definition is here } arg_bl; ^ thanks -- PMM
Re: [Qemu-devel] [PATCH v2] virtio-blk: set correct config size for the host driver
On Tue, Feb 12, 2019 at 02:01:44PM +0800, Changpeng Liu wrote:
> Commit caa1ee43 "vhost-user-blk: add discard/write zeroes features support"
> introduced extra fields to existing struct virtio_blk_config, when
> migration was executed from older QEMU version to current head, it
> will break the migration. While here, set the correct config size
> when initializing the host driver, for now, discard/write zeroes
> are not supported by virtio-blk host driver, so set the config
> size as before, users can change config size when adding the new
> feature bits support.
>
> Signed-off-by: Changpeng Liu
Pls rewrite commit log as suggested in this thread
and repost.
> ---
> hw/block/virtio-blk.c | 10 ++
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
> index 9a87b3b..846b7b9 100644
> --- a/hw/block/virtio-blk.c
> +++ b/hw/block/virtio-blk.c
> @@ -28,6 +28,9 @@
> #include "hw/virtio/virtio-bus.h"
> #include "hw/virtio/virtio-access.h"
>
> +#define VIRTIO_BLK_CFG_SIZE (offsetof(struct virtio_blk_config, num_queues)
> + \
> + sizeof_field(struct virtio_blk_config,
> num_queues))
> +
I would just do offsetof(max_discard_sectors)
with a comment "we don't support discard yet, hide associated config
fields".
> static void virtio_blk_init_request(VirtIOBlock *s, VirtQueue *vq,
> VirtIOBlockReq *req)
> {
> @@ -761,7 +764,7 @@ static void virtio_blk_update_config(VirtIODevice *vdev,
> uint8_t *config)
> blkcfg.alignment_offset = 0;
> blkcfg.wce = blk_enable_write_cache(s->blk);
> virtio_stw_p(vdev, _queues, s->conf.num_queues);
> -memcpy(config, , sizeof(struct virtio_blk_config));
> +memcpy(config, , VIRTIO_BLK_CFG_SIZE);
Let's add QEMU_BUILD_BUG_ON(VIRTIO_BLK_CFG_SIZE <= sizeof(struct
virtio_blk_config)) just for documentation purposes.
> }
>
> static void virtio_blk_set_config(VirtIODevice *vdev, const uint8_t *config)
> @@ -769,7 +772,7 @@ static void virtio_blk_set_config(VirtIODevice *vdev,
> const uint8_t *config)
> VirtIOBlock *s = VIRTIO_BLK(vdev);
> struct virtio_blk_config blkcfg;
>
> -memcpy(, config, sizeof(blkcfg));
> +memcpy(, config, VIRTIO_BLK_CFG_SIZE);
Here too, QEMU_BUILD_BUG_ON(VIRTIO_BLK_CFG_SIZE <= sizeof(blkcfg))
>
> aio_context_acquire(blk_get_aio_context(s->blk));
> blk_set_enable_write_cache(s->blk, blkcfg.wce != 0);
> @@ -952,8 +955,7 @@ static void virtio_blk_device_realize(DeviceState *dev,
> Error **errp)
> return;
> }
>
> -virtio_init(vdev, "virtio-blk", VIRTIO_ID_BLOCK,
> -sizeof(struct virtio_blk_config));
> +virtio_init(vdev, "virtio-blk", VIRTIO_ID_BLOCK, VIRTIO_BLK_CFG_SIZE);
>
> s->blk = conf->conf.blk;
> s->rq = NULL;
> --
> 1.9.3
[Qemu-devel] [RFC PULL 2/6] virtio-blk: add host_features field in VirtIOBlock
From: Stefano Garzarella
Since configurable features for virtio-blk are growing, this patch
adds host_features field in the struct VirtIOBlock. (as in virtio-net)
In this way, we can avoid to add new fields for new properties and
we can directly set VIRTIO_BLK_F* flags in the host_features.
We update "config-wce" and "scsi" property definition to use the new
host_features field without change the behaviour.
Suggested-by: Michael S. Tsirkin
Reviewed-by: Stefan Hajnoczi
Signed-off-by: Stefano Garzarella
Acked-by: Pankaj Gupta
Message-id: 20190208134950.187665-3-sgarz...@redhat.com
Signed-off-by: Stefan Hajnoczi
---
include/hw/virtio/virtio-blk.h | 3 +--
hw/block/virtio-blk.c | 16 +---
2 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h
index 5117431d96..f7345b0511 100644
--- a/include/hw/virtio/virtio-blk.h
+++ b/include/hw/virtio/virtio-blk.h
@@ -35,8 +35,6 @@ struct VirtIOBlkConf
BlockConf conf;
IOThread *iothread;
char *serial;
-uint32_t scsi;
-uint32_t config_wce;
uint32_t request_merging;
uint16_t num_queues;
uint16_t queue_size;
@@ -57,6 +55,7 @@ typedef struct VirtIOBlock {
bool dataplane_disabled;
bool dataplane_started;
struct VirtIOBlockDataPlane *dataplane;
+uint64_t host_features;
} VirtIOBlock;
typedef struct VirtIOBlockReq {
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index b2bb129efa..938273c63c 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -242,7 +242,7 @@ static int virtio_blk_handle_scsi_req(VirtIOBlockReq *req)
*/
scsi = (void *)elem->in_sg[elem->in_num - 2].iov_base;
-if (!blk->conf.scsi) {
+if (!virtio_has_feature(blk->host_features, VIRTIO_BLK_F_SCSI)) {
status = VIRTIO_BLK_S_UNSUPP;
goto fail;
}
@@ -783,12 +783,15 @@ static uint64_t virtio_blk_get_features(VirtIODevice
*vdev, uint64_t features,
{
VirtIOBlock *s = VIRTIO_BLK(vdev);
+/* Firstly sync all virtio-blk possible supported features */
+features |= s->host_features;
+
virtio_add_feature(, VIRTIO_BLK_F_SEG_MAX);
virtio_add_feature(, VIRTIO_BLK_F_GEOMETRY);
virtio_add_feature(, VIRTIO_BLK_F_TOPOLOGY);
virtio_add_feature(, VIRTIO_BLK_F_BLK_SIZE);
if (virtio_has_feature(features, VIRTIO_F_VERSION_1)) {
-if (s->conf.scsi) {
+if (virtio_has_feature(s->host_features, VIRTIO_BLK_F_SCSI)) {
error_setg(errp, "Please set scsi=off for virtio-blk devices in
order to use virtio 1.0");
return 0;
}
@@ -797,9 +800,6 @@ static uint64_t virtio_blk_get_features(VirtIODevice *vdev,
uint64_t features,
virtio_add_feature(, VIRTIO_BLK_F_SCSI);
}
-if (s->conf.config_wce) {
-virtio_add_feature(, VIRTIO_BLK_F_CONFIG_WCE);
-}
if (blk_enable_write_cache(s->blk)) {
virtio_add_feature(, VIRTIO_BLK_F_WCE);
}
@@ -1014,9 +1014,11 @@ static Property virtio_blk_properties[] = {
DEFINE_BLOCK_ERROR_PROPERTIES(VirtIOBlock, conf.conf),
DEFINE_BLOCK_CHS_PROPERTIES(VirtIOBlock, conf.conf),
DEFINE_PROP_STRING("serial", VirtIOBlock, conf.serial),
-DEFINE_PROP_BIT("config-wce", VirtIOBlock, conf.config_wce, 0, true),
+DEFINE_PROP_BIT64("config-wce", VirtIOBlock, host_features,
+ VIRTIO_BLK_F_CONFIG_WCE, true),
#ifdef __linux__
-DEFINE_PROP_BIT("scsi", VirtIOBlock, conf.scsi, 0, false),
+DEFINE_PROP_BIT64("scsi", VirtIOBlock, host_features,
+ VIRTIO_BLK_F_SCSI, false),
#endif
DEFINE_PROP_BIT("request-merging", VirtIOBlock, conf.request_merging, 0,
true),
--
2.20.1
[Qemu-devel] [RFC PULL 0/6] Block pull request for testing
Peter hit a virtio-blk-test failure caused by the new DISCARD/WRITE_ZEROES patches that Stefano and I have been unable to reproduce. Here are the patches so they can be tested again in Peter's environment. Stefano Garzarella (6): virtio-blk: add acct_failed param to virtio_blk_handle_rw_error() virtio-blk: add host_features field in VirtIOBlock virtio-blk: add "discard" and "write-zeroes" properties virtio-blk: add DISCARD and WRITE_ZEROES features tests/virtio-blk: change assert on data_size in virtio_blk_request() tests/virtio-blk: add test for WRITE_ZEROES command include/hw/virtio/virtio-blk.h | 5 +- hw/block/virtio-blk.c | 214 +++-- hw/core/machine.c | 2 + tests/virtio-blk-test.c| 75 +++- 4 files changed, 282 insertions(+), 14 deletions(-) -- 2.20.1
Re: [Qemu-devel] [PATCH 0/4] a few trivials
On Tue, Feb 12, 2019 at 01:47:54PM +, Dr. David Alan Gilbert (git) wrote: > From: "Dr. David Alan Gilbert" > > Just cleaning out my trivial pile; a few more traces and > a couple of error reporting tweaks. > > Dave Looks sane Reviewed-by: Michael S. Tsirkin > > Dr. David Alan Gilbert (4): > pckbd: Convert DPRINTF->trace > HMP: Prepend errors with 'Error:' > kvm: Add kvm_set_ioeventfd* traces > wavcapture: Convert to error_report > > accel/kvm/kvm-all.c| 3 +++ > accel/kvm/trace-events | 2 ++ > audio/wavcapture.c | 39 +-- > hmp.c | 2 +- > hw/input/pckbd.c | 19 ++- > hw/input/trace-events | 7 +++ > 6 files changed, 36 insertions(+), 36 deletions(-) > > -- > 2.20.1
[Qemu-devel] [PATCH 2/3] contrib/vhost-user-blk: fix the compilation issue
From: Changpeng Liu
Signed-off-by: Changpeng Liu
Reviewed-by: Stefan Hajnoczi
Reviewed-by: Stefano Garzarella
Reviewed-by: Michael S. Tsirkin
Message-Id: <1547615970-23545-2-git-send-email-changpeng@intel.com>
[PMD: this patch was first (incorrectly) introduced as a56de056c91f8]
Signed-off-by: Philippe Mathieu-Daudé
---
contrib/vhost-user-blk/vhost-user-blk.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/contrib/vhost-user-blk/vhost-user-blk.c
b/contrib/vhost-user-blk/vhost-user-blk.c
index 5c2092e13a..43583f2659 100644
--- a/contrib/vhost-user-blk/vhost-user-blk.c
+++ b/contrib/vhost-user-blk/vhost-user-blk.c
@@ -20,6 +20,10 @@
#include "contrib/libvhost-user/libvhost-user-glib.h"
#include "contrib/libvhost-user/libvhost-user.h"
+#if defined(__linux__)
+#include
+#include
+#endif
struct virtio_blk_inhdr {
unsigned char status;
@@ -521,7 +525,7 @@ vub_get_blocksize(int fd)
#if defined(__linux__) && defined(BLKSSZGET)
if (ioctl(fd, BLKSSZGET, ) == 0) {
-return blocklen;
+return blocksize;
}
#endif
--
2.20.1
Re: [Qemu-devel] [PATCH 3/4] kvm: Add kvm_set_ioeventfd* traces
On 2/12/19 2:47 PM, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert"
>
> Add a couple of traces around the kvm_set_ioeventfd* calls.
>
> Signed-off-by: Dr. David Alan Gilbert
Reviewed-by: Philippe Mathieu-Daudé
> ---
> accel/kvm/kvm-all.c| 3 +++
> accel/kvm/trace-events | 2 ++
> 2 files changed, 5 insertions(+)
>
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index 4e1de942ce..fd92b6f375 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -657,6 +657,8 @@ static int kvm_set_ioeventfd_mmio(int fd, hwaddr addr,
> uint32_t val,
> .fd = fd,
> };
>
> +trace_kvm_set_ioeventfd_mmio(fd, (uint64_t)addr, val, assign, size,
> + datamatch);
> if (!kvm_enabled()) {
> return -ENOSYS;
> }
> @@ -688,6 +690,7 @@ static int kvm_set_ioeventfd_pio(int fd, uint16_t addr,
> uint16_t val,
> .fd = fd,
> };
> int r;
> +trace_kvm_set_ioeventfd_pio(fd, addr, val, assign, size, datamatch);
> if (!kvm_enabled()) {
> return -ENOSYS;
> }
> diff --git a/accel/kvm/trace-events b/accel/kvm/trace-events
> index 58e98efe5d..8841025d68 100644
> --- a/accel/kvm/trace-events
> +++ b/accel/kvm/trace-events
> @@ -12,5 +12,7 @@ kvm_irqchip_commit_routes(void) ""
> kvm_irqchip_add_msi_route(char *name, int vector, int virq) "dev %s vector
> %d virq %d"
> kvm_irqchip_update_msi_route(int virq) "Updating MSI route virq=%d"
> kvm_irqchip_release_virq(int virq) "virq %d"
> +kvm_set_ioeventfd_mmio(int fd, uint64_t addr, uint32_t val, bool assign,
> uint32_t size, bool datamatch) "fd: %d @0x%" PRIx64 " val=0x%x assign: %d
> size: %d match: %d"
> +kvm_set_ioeventfd_pio(int fd, uint16_t addr, uint32_t val, bool assign,
> uint32_t size, bool datamatch) "fd: %d @0x%x val=0x%x assign: %d size: %d
> match: %d"
> kvm_set_user_memory(uint32_t slot, uint32_t flags, uint64_t guest_phys_addr,
> uint64_t memory_size, uint64_t userspace_addr, int ret) "Slot#%d flags=0x%x
> gpa=0x%"PRIx64 " size=0x%"PRIx64 " ua=0x%"PRIx64 " ret=%d"
>
>
[Qemu-devel] [RFC PULL 1/6] virtio-blk: add acct_failed param to virtio_blk_handle_rw_error()
From: Stefano Garzarella
We add acct_failed param in order to use virtio_blk_handle_rw_error()
also when is not required to call block_acct_failed(). (eg. a discard
operation is failed)
Reviewed-by: Michael S. Tsirkin
Reviewed-by: Stefan Hajnoczi
Signed-off-by: Stefano Garzarella
Acked-by: Pankaj Gupta
Message-id: 20190208134950.187665-2-sgarz...@redhat.com
Signed-off-by: Stefan Hajnoczi
---
hw/block/virtio-blk.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 9a87b3bfac..b2bb129efa 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -61,7 +61,7 @@ static void virtio_blk_req_complete(VirtIOBlockReq *req,
unsigned char status)
}
static int virtio_blk_handle_rw_error(VirtIOBlockReq *req, int error,
-bool is_read)
+bool is_read, bool acct_failed)
{
BlockErrorAction action = blk_get_error_action(req->dev->blk,
is_read, error);
@@ -75,7 +75,9 @@ static int virtio_blk_handle_rw_error(VirtIOBlockReq *req,
int error,
s->rq = req;
} else if (action == BLOCK_ERROR_ACTION_REPORT) {
virtio_blk_req_complete(req, VIRTIO_BLK_S_IOERR);
-block_acct_failed(blk_get_stats(s->blk), >acct);
+if (acct_failed) {
+block_acct_failed(blk_get_stats(s->blk), >acct);
+}
virtio_blk_free_request(req);
}
@@ -113,7 +115,7 @@ static void virtio_blk_rw_complete(void *opaque, int ret)
* the memory until the request is completed (which will
* happen on the other side of the migration).
*/
-if (virtio_blk_handle_rw_error(req, -ret, is_read)) {
+if (virtio_blk_handle_rw_error(req, -ret, is_read, true)) {
continue;
}
}
@@ -132,7 +134,7 @@ static void virtio_blk_flush_complete(void *opaque, int ret)
aio_context_acquire(blk_get_aio_context(s->conf.conf.blk));
if (ret) {
-if (virtio_blk_handle_rw_error(req, -ret, 0)) {
+if (virtio_blk_handle_rw_error(req, -ret, 0, true)) {
goto out;
}
}
--
2.20.1
[Qemu-devel] [PATCH v3] virtio-blk: set correct config size for the host driver
Commit caa1ee43 "vhost-user-blk: add discard/write zeroes features
support" added fields to struct virtio_blk_config. This changes
the size of the config space and breaks migration from QEMU 3.1
and older:
qemu-system-ppc64: get_pci_config_device: Bad config data: i=0x10 read: 41
device: 1 cmask: ff wmask: 80 w1cmask:0
qemu-system-ppc64: Failed to load PCIDevice:config
qemu-system-ppc64: Failed to load virtio-blk:virtio
qemu-system-ppc64: error while loading state for instance 0x0 of device
'pci@8002000:01.0/virtio-blk'
qemu-system-ppc64: load of migration failed: Invalid argument
Since virtio-blk doesn't support the "discard" and "write zeroes"
features, it shouldn't even expose the associated fields in the
config space actually. Just include all fields up to num_queues to
match QEMU 3.1 and older.
Signed-off-by: Changpeng Liu
---
hw/block/virtio-blk.c | 14 ++
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 9a87b3b..0ff5315 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -28,6 +28,10 @@
#include "hw/virtio/virtio-bus.h"
#include "hw/virtio/virtio-access.h"
+/* We don't support discard yet, hide associated config fields. */
+#define VIRTIO_BLK_CFG_SIZE offsetof(struct virtio_blk_config, \
+ max_discard_sectors)
+
static void virtio_blk_init_request(VirtIOBlock *s, VirtQueue *vq,
VirtIOBlockReq *req)
{
@@ -761,7 +765,9 @@ static void virtio_blk_update_config(VirtIODevice *vdev,
uint8_t *config)
blkcfg.alignment_offset = 0;
blkcfg.wce = blk_enable_write_cache(s->blk);
virtio_stw_p(vdev, _queues, s->conf.num_queues);
-memcpy(config, , sizeof(struct virtio_blk_config));
+memcpy(config, , VIRTIO_BLK_CFG_SIZE);
+QEMU_BUILD_BUG_ON(VIRTIO_BLK_CFG_SIZE > sizeof(struct virtio_blk_config));
+
}
static void virtio_blk_set_config(VirtIODevice *vdev, const uint8_t *config)
@@ -769,7 +775,8 @@ static void virtio_blk_set_config(VirtIODevice *vdev, const
uint8_t *config)
VirtIOBlock *s = VIRTIO_BLK(vdev);
struct virtio_blk_config blkcfg;
-memcpy(, config, sizeof(blkcfg));
+memcpy(, config, VIRTIO_BLK_CFG_SIZE);
+QEMU_BUILD_BUG_ON(VIRTIO_BLK_CFG_SIZE > sizeof(blkcfg));
aio_context_acquire(blk_get_aio_context(s->blk));
blk_set_enable_write_cache(s->blk, blkcfg.wce != 0);
@@ -952,8 +959,7 @@ static void virtio_blk_device_realize(DeviceState *dev,
Error **errp)
return;
}
-virtio_init(vdev, "virtio-blk", VIRTIO_ID_BLOCK,
-sizeof(struct virtio_blk_config));
+virtio_init(vdev, "virtio-blk", VIRTIO_ID_BLOCK, VIRTIO_BLK_CFG_SIZE);
s->blk = conf->conf.blk;
s->rq = NULL;
--
1.9.3
Re: [Qemu-devel] [PATCH] s390x/kvm: add tracepoint to ioeventfd interface
Hi Cornelia,
On 2/12/19 4:30 PM, Cornelia Huck wrote:
> Trace when assigning/unassigning.
>
> Signed-off-by: Cornelia Huck
> ---
> target/s390x/kvm.c| 2 ++
> target/s390x/trace-events | 1 +
> 2 files changed, 3 insertions(+)
>
> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> index 15fdc168e1c5..7d61bd109092 100644
> --- a/target/s390x/kvm.c
> +++ b/target/s390x/kvm.c
> @@ -1886,6 +1886,8 @@ int kvm_s390_assign_subch_ioeventfd(EventNotifier
> *notifier, uint32_t sch,
> .addr = sch,
> .len = 8,
> };
> +trace_kvm_s390_assign_subch_ioeventfd(kick.fd, kick.addr, assign,
> + kick.datamatch);
> if (!kvm_check_extension(kvm_state, KVM_CAP_IOEVENTFD)) {
> return -ENOSYS;
> }
> diff --git a/target/s390x/trace-events b/target/s390x/trace-events
> index a84e316e4937..bdc22f42cdbb 100644
> --- a/target/s390x/trace-events
> +++ b/target/s390x/trace-events
> @@ -14,6 +14,7 @@ ioinst_chsc_cmd(uint16_t cmd, uint16_t len) "IOINST: chsc
> command 0x%04x, len 0x
> kvm_enable_cmma(int rc) "CMMA: enabling with result code %d"
> kvm_clear_cmma(int rc) "CMMA: clearing with result code %d"
> kvm_failed_cpu_state_set(int cpu_index, uint8_t state, const char *msg)
> "Warning: Unable to set cpu %d state %" PRIu8 " to KVM: %s"
> +kvm_s390_assign_subch_ioeventfd(int fd, uint32_t addr, bool assign, int
> datamatch) "fd: %d sch: @0x%x assign: %d vq: %d"
I noticed all s390x related trace events don't specify 's390' in the
event name, maybe you can simply strip it. If you agree, feel free to
apply that change directly yourself, no need for v2 ;)
Regardless the name used:
Reviewed-by: Philippe Mathieu-Daudé
>
> # target/s390x/cpu.c
> cpu_set_state(int cpu_index, uint8_t state) "setting cpu %d state to %" PRIu8
>
Re: [Qemu-devel] [PATCH 23/25] hw/arm: Express dependencies of the ZynqMP zcu102 machine with Kconfig
On 2019-02-11 15:45, Paolo Bonzini wrote: > On 09/02/19 07:39, Thomas Huth wrote: >> +select PCI # TODO: Currently required for SDHCI and AHCI, remove >> later > > I think SDHCI and AHCI should select it instead, similar to how FDC > selects ISA_BUS (see patch 25 of the Kconfig series). Yes, that's likely the better way to work-around this issue. I'll add that in v2 ... Thomas
Re: [Qemu-devel] [PATCH v3 1/2] pcie: Add a simple PCIe ACS (Access Control Services) helper function
On Tue, 2019-02-12 at 08:59 -0700, Alex Williamson wrote:
> On Tue, 12 Feb 2019 09:07:43 +0100
> Knut Omang wrote:
>
> > On Mon, 2019-02-11 at 16:09 -0700, Alex Williamson wrote:
> > > On Sun, 10 Feb 2019 07:52:59 +0100
> > > Knut Omang wrote:
> > >
> > > > Add a helper function to add PCIe capability for Access Control Services
> > > > (ACS)
> > > > ACS support in the associated root port is a prerequisite to be able to
> > > > do
> > > > passthrough of individual functions of a device with VFIO
> > > > without Alex Williamson's pcie_acs_override kernel patch or similar
> > > > in the guest.
> > >
> > > This is still incorrect, the ACS override patch is only required for
> > > separating multifunction endpoints or multifunction root ports. Single
> > > function endpoints are assignable without ACS simply by placing them
> > > downstream of a single function root port or directly on the root
> > > complex.
> >
> > Hmm - that was the intended meaning of the comment, but I'll see if I can
> > make
> > it more clear by saying it explicitly.
>
> "ACS support... is a prerequisite". Prerequisite: a thing that is
> required as a prior condition for something else to happen or exist.
>
> Assignment of individual functions exists today, as is, by using QEMU
> to define a PCIe topology that allows the desired grouping. The code
> here enables specific topologies, it is clearly not a prerequisite.
>
> > > > Endpoints may also implement an ACS capability, but with
> > > > limited features.
> > > >
> > > > Signed-off-by: Knut Omang
> > > > ---
> > > > hw/pci/pcie.c | 29 +
> > > > include/hw/pci/pcie.h | 6 ++
> > > > include/hw/pci/pcie_regs.h | 4
> > > > 3 files changed, 39 insertions(+)
> > > >
> > > > diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> > > > index 230478f..509632f 100644
> > > > --- a/hw/pci/pcie.c
> > > > +++ b/hw/pci/pcie.c
> > > > @@ -742,6 +742,14 @@ bool pcie_cap_is_arifwd_enabled(const PCIDevice
> > > > *dev)
> > > > PCI_EXP_DEVCTL2_ARI;
> > > > }
> > > >
> > > > +/* Access Control Services (ACS) */
> > > > +void pcie_cap_acs_reset(PCIDevice *dev)
> > > > +{
> > > > +if (dev->exp.acs_cap) {
> > > > +pci_set_word(dev->config + dev->exp.acs_cap + PCI_ACS_CTRL, 0);
> > > > +}
> > > > +}
> > > > +
> > > > /**
> > > >
> > > > * pci express extended capability list management functions
> > > > * uint16_t ext_cap_id (16 bit)
> > > > @@ -906,3 +914,24 @@ void pcie_ats_init(PCIDevice *dev, uint16_t offset)
> > > >
> > > > pci_set_word(dev->wmask + dev->exp.ats_cap + PCI_ATS_CTRL, 0x800f);
> > > > }
> > > > +
> > > > +/* ACS (Access Control Services) */
> > > > +void pcie_acs_init(PCIDevice *dev, uint16_t offset)
> > > > +{
> > > > +bool is_pcie_slot = !!object_dynamic_cast(OBJECT(dev),
> > > > TYPE_PCIE_SLOT);
> > > > +uint16_t cap_bits = 0;
> > > > +
> > > > +pcie_add_capability(dev, PCI_EXT_CAP_ID_ACS, PCI_ACS_VER, offset,
> > > > +PCI_ACS_SIZEOF);
> > > > +dev->exp.acs_cap = offset;
> > > > +
> > > > +if (is_pcie_slot) {
> > > > +/* Endpoints may also implement ACS, but these capabilities are
> > > > */
> > > > +/* only valid for slots: */
> > >
> > > Not quite, SV, TB, and UF must not be implemented by endpoints, but RR
> > > and CR must be implemented by multifunction endpoints that support p2p
> > > if they provide an ACS capability.
> >
> > Hmm - are you ok with setting 0 here as I have done, just amending your
> > description to the comment? Then any future emulation that do support p2p
> > would have to set the needed bits after calling the init function.
>
> The comment definitely needs work, but I don't know what to do about
> single function, non-SR-IOV capable devices calling into this.
I agree - I have only been thinking about multifunction devices, I should
probably assert on not multifunction (or SR/IOV with my SR/IOV patch set)
to avoid misuse.
And what about someone passing a PF for SR/IOV through to a VM -
Has that use case showed up (yet) ?
I have so far only tested with my emulated SR/IOV.
> Per the
> spec, these devices cannot implement an ACS capability at all, but will
> anyone care if they do? Maybe endpoints need a different init
> function. Maybe the capability ID needs to be obscured until
> additional functions or an SR-IOV capability are added. I'm not really
> concerned that this helper can only indicate lack of p2p between
> functions for endpoints, other than the comments being wrong.
Ok, I think I am in the clear then with my v4 comment :-)
> > After your previous comments on this, I had a look at Mellanox CX4 and CX5
> > which
> > are the only devices I could find in the lab that are endpoints and
> > implement an
> > ACS capability, and neither seems to implement any extra capabilities:
> >
> >
Re: [Qemu-devel] [PULL 00/25] pci, pc, virtio: fixes, cleanups, features
On 2/12/19 2:04 PM, Michael S. Tsirkin wrote: > On Tue, Feb 12, 2019 at 11:39:21AM +0100, Philippe Mathieu-Daudé wrote: >> On 2/12/19 8:11 AM, Peter Xu wrote: >>> On Tue, Feb 05, 2019 at 11:06:42AM -0500, Michael S. Tsirkin wrote: OK I reverted the whole part dealing with vhost-user and reposted. >>> >>> I noticed that the merged pull request could possibly have squashed >>> the below two patches (in previous pull) into one super patch >>> (a56de056c91f87e1e): >>> >>> i386/kvm: ignore masked irqs when update msi routes >>> contrib/vhost-user-blk: fix the compilation issue >>> >>> Here, the first patch lost its commit message, and the last patch lost >>> its real author. ;) >> >> I suggest we revert a56de056c9 ASAP and reapply the both patches, this >> will ease cherry-picking/downstream workflow. > > I don't see why does it help upstream. I'd have suggested the same if I had no idea what 'downstream workflow' mean, simply to keep the tree clear and avoid to have unrelated changes squashed altogether. Commit a56de056c9 really looks messy. MSI/MSIX changes described by "fix vhost-user-blk compilation". Hopefully it won't trigger any problem which requires bisecting to it, then contact Changpeng Liu asking him what he intented to do with his commit. Your call anyway :) Regards, Phil.
[Qemu-devel] [PATCH 1/3] Revert "contrib/vhost-user-blk: fix the compilation issue"
Commit a56de056c91f8 squashed the following two unrelated commits
at once:
- "contrib/vhost-user-blk: fix the compilation issue"
(Message-Id: 1547615970-23545-2-git-send-email-changpeng@intel.com)
- "i386/kvm: ignore masked irqs when update msi routes"
(Message-Id: 20190116030815.27273-5-pet...@redhat.com)
While the git history remains bisectable, having a commit that changes
MSI/MSIX code but describes it as "fix vhost-user-blk compilation" is
rather confusing.
Revert the offending commit to properly apply both patches separately.
Reported-by: Peter Xu
Fixes: a56de056c91f8
Signed-off-by: Philippe Mathieu-Daudé
---
contrib/vhost-user-blk/vhost-user-blk.c | 6 +-
target/i386/kvm.c | 14 +++---
2 files changed, 4 insertions(+), 16 deletions(-)
diff --git a/contrib/vhost-user-blk/vhost-user-blk.c
b/contrib/vhost-user-blk/vhost-user-blk.c
index 43583f2659..5c2092e13a 100644
--- a/contrib/vhost-user-blk/vhost-user-blk.c
+++ b/contrib/vhost-user-blk/vhost-user-blk.c
@@ -20,10 +20,6 @@
#include "contrib/libvhost-user/libvhost-user-glib.h"
#include "contrib/libvhost-user/libvhost-user.h"
-#if defined(__linux__)
-#include
-#include
-#endif
struct virtio_blk_inhdr {
unsigned char status;
@@ -525,7 +521,7 @@ vub_get_blocksize(int fd)
#if defined(__linux__) && defined(BLKSSZGET)
if (ioctl(fd, BLKSSZGET, ) == 0) {
-return blocksize;
+return blocklen;
}
#endif
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index beae1b99da..9af4542fb8 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -3894,7 +3894,7 @@ static QLIST_HEAD(, MSIRouteEntry) msi_route_list = \
static void kvm_update_msi_routes_all(void *private, bool global,
uint32_t index, uint32_t mask)
{
-int cnt = 0, vector;
+int cnt = 0;
MSIRouteEntry *entry;
MSIMessage msg;
PCIDevice *dev;
@@ -3902,19 +3902,11 @@ static void kvm_update_msi_routes_all(void *private,
bool global,
/* TODO: explicit route update */
QLIST_FOREACH(entry, _route_list, list) {
cnt++;
-vector = entry->vector;
dev = entry->dev;
-if (msix_enabled(dev) && !msix_is_masked(dev, vector)) {
-msg = msix_get_message(dev, vector);
-} else if (msi_enabled(dev) && !msi_is_masked(dev, vector)) {
-msg = msi_get_message(dev, vector);
-} else {
-/*
- * Either MSI/MSIX is disabled for the device, or the
- * specific message was masked out. Skip this one.
- */
+if (!msix_enabled(dev) && !msi_enabled(dev)) {
continue;
}
+msg = pci_get_msi_message(dev, entry->vector);
kvm_irqchip_update_msi_route(kvm_state, entry->virq, msg, dev);
}
kvm_irqchip_commit_routes(kvm_state);
--
2.20.1
[Qemu-devel] [RFC PULL 5/6] tests/virtio-blk: change assert on data_size in virtio_blk_request()
From: Stefano Garzarella
The size of data in the virtio_blk_request must be a multiple
of 512 bytes for IN and OUT requests, or a multiple of the size
of struct virtio_blk_discard_write_zeroes for DISCARD and
WRITE_ZEROES requests.
Reviewed-by: Michael S. Tsirkin
Reviewed-by: Stefan Hajnoczi
Reviewed-by: Thomas Huth
Signed-off-by: Stefano Garzarella
Acked-by: Pankaj Gupta
Message-id: 20190208134950.187665-6-sgarz...@redhat.com
Signed-off-by: Stefan Hajnoczi
---
tests/virtio-blk-test.c | 15 ++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
index 04c608764b..0739498da7 100644
--- a/tests/virtio-blk-test.c
+++ b/tests/virtio-blk-test.c
@@ -144,7 +144,20 @@ static uint64_t virtio_blk_request(QGuestAllocator *alloc,
QVirtioDevice *d,
uint64_t addr;
uint8_t status = 0xFF;
-g_assert_cmpuint(data_size % 512, ==, 0);
+switch (req->type) {
+case VIRTIO_BLK_T_IN:
+case VIRTIO_BLK_T_OUT:
+g_assert_cmpuint(data_size % 512, ==, 0);
+break;
+case VIRTIO_BLK_T_DISCARD:
+case VIRTIO_BLK_T_WRITE_ZEROES:
+g_assert_cmpuint(data_size %
+ sizeof(struct virtio_blk_discard_write_zeroes), ==,
0);
+break;
+default:
+g_assert_cmpuint(data_size, ==, 0);
+}
+
addr = guest_alloc(alloc, sizeof(*req) + data_size);
virtio_blk_fix_request(d, req);
--
2.20.1
Re: [Qemu-devel] [PATCH 3/4] kvm: Add kvm_set_ioeventfd* traces
On Tue, 12 Feb 2019 13:47:57 + "Dr. David Alan Gilbert (git)" wrote: > From: "Dr. David Alan Gilbert" > > Add a couple of traces around the kvm_set_ioeventfd* calls. > > Signed-off-by: Dr. David Alan Gilbert > --- > accel/kvm/kvm-all.c| 3 +++ > accel/kvm/trace-events | 2 ++ > 2 files changed, 5 insertions(+) Reviewed-by: Cornelia Huck Maybe I'll add a tracepoint for the s390 subchannel ioeventfd as well...
Re: [Qemu-devel] [PATCH v2 3/3] tpm_tis: fix format string specifier in tpm_tis_show_buffer()
On 2/12/19 2:43 PM, Liam Merwick wrote:
> On 12/02/2019 13:27, Stefan Berger wrote:
>> On 2/12/19 7:31 AM, Philippe Mathieu-Daudé wrote:
>>> On 2/11/19 10:13 PM, Stefan Berger wrote:
On 2/11/19 3:09 PM, Liam Merwick wrote:
> On 11/02/2019 19:56, Stefan Berger wrote:
>> On 2/11/19 11:02 AM, Philippe Mathieu-Daudé wrote:
>>> On 2/11/19 4:03 PM, Liam Merwick wrote:
>>> [...]
- printf("tpm_tis: %s length = %d\n", string, len);
+ printf("tpm_tis: %s length = %u\n", string, len);
>>> So here the format is '%zu'.
>>> However in code cleanup we try go get ride of printf() calls and
>>> replace them with trace points.
>>
>> This code is only used for debugging if DEBUG_TIS has been #defined.
>> No need to add tracing here.
> I'd come up the attached change (but that seems like overkill).
I don't think we need tracing for this.
>>> So if you think the code is mature enough, let's remove the DEBUG calls!
>>>
>>> Else we prefer to convert DEBUG printf to trace events because (at
>>> least):
>>> - no need to recompile to enable debugging
>>> - when compiled with debugging, you don't mess with STDIO which can be
>>> used as a chardev backend.
>> Fine. Then I withdraw my reviewed-by.
>
>
> I don't see a way of removing the DEBUG calls without adding the
> overhead of a call to tpm_tis_show_buffer() each time even if tracing is
> not enabled (the 3 trace calls are interdependent and need a for loop).
> Is there any example of a trace point that calls a function that then
> does non-trivial printing?
>
> I could send a v3 with the patch I attached previously with the 3 printf
> calls changed to trace points but still wrapped in 'if (DEBUG_TIS)' and
> optimised out in non-debug.
You can look at commit 56853498648.
Thanks!
Phil.
Re: [Qemu-devel] [v3 PATCH] hw/arm/bcm2835_peripherals: add bcm283x sp804-alike timer
Hi Andrew, Thanks for reviewing the patch I've submitted. I've already messed up two and this is my first time contributing to any open-source software. ☺ I've checked the implementation you suggested and I think it would be a great example for me to implement free-running counters, too. Best regards, Mark
[Qemu-devel] [PATCH] s390x/kvm: add tracepoint to ioeventfd interface
Trace when assigning/unassigning.
Signed-off-by: Cornelia Huck
---
target/s390x/kvm.c| 2 ++
target/s390x/trace-events | 1 +
2 files changed, 3 insertions(+)
diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index 15fdc168e1c5..7d61bd109092 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -1886,6 +1886,8 @@ int kvm_s390_assign_subch_ioeventfd(EventNotifier
*notifier, uint32_t sch,
.addr = sch,
.len = 8,
};
+trace_kvm_s390_assign_subch_ioeventfd(kick.fd, kick.addr, assign,
+ kick.datamatch);
if (!kvm_check_extension(kvm_state, KVM_CAP_IOEVENTFD)) {
return -ENOSYS;
}
diff --git a/target/s390x/trace-events b/target/s390x/trace-events
index a84e316e4937..bdc22f42cdbb 100644
--- a/target/s390x/trace-events
+++ b/target/s390x/trace-events
@@ -14,6 +14,7 @@ ioinst_chsc_cmd(uint16_t cmd, uint16_t len) "IOINST: chsc
command 0x%04x, len 0x
kvm_enable_cmma(int rc) "CMMA: enabling with result code %d"
kvm_clear_cmma(int rc) "CMMA: clearing with result code %d"
kvm_failed_cpu_state_set(int cpu_index, uint8_t state, const char *msg)
"Warning: Unable to set cpu %d state %" PRIu8 " to KVM: %s"
+kvm_s390_assign_subch_ioeventfd(int fd, uint32_t addr, bool assign, int
datamatch) "fd: %d sch: @0x%x assign: %d vq: %d"
# target/s390x/cpu.c
cpu_set_state(int cpu_index, uint8_t state) "setting cpu %d state to %" PRIu8
--
2.17.2
[Qemu-devel] [PATCH 1/2] slirp: remove slirp_ prefix for socket wrappers
QEMU wraps the socket functions in os-win32.h, but in commit
a9d8b3ec4385793815d71217857304, the header inclusion was dropped,
breaking libslirp on Windows.
There are already a few socket functions that are wrapped in libslirp,
with "slirp_" prefix, but many of them are missing, and we are going
to wrap the missing functions in a second patch.
Using "slirp_" prefix avoids the conflict with socket function #define
wrappers in QEMU os-win32.h, but they are quite intrusive. In the end,
the functions should behave the same as original one, but with errno
being set. To avoid the churn, and potential confusion, remove the
"slirp_" prefix. A series of #undef is necessary until libslirp is
made standalone to prevent the #define conflict with QEMU.
Signed-off-by: Marc-André Lureau
---
slirp/util.h | 31 ---
slirp/ip_icmp.c | 4 ++--
slirp/misc.c | 14 +++---
slirp/slirp.c| 2 +-
slirp/socket.c | 14 +++---
slirp/tcp_subr.c | 10 +-
slirp/udp.c | 2 +-
7 files changed, 43 insertions(+), 34 deletions(-)
diff --git a/slirp/util.h b/slirp/util.h
index 4963747aef..685b5af099 100644
--- a/slirp/util.h
+++ b/slirp/util.h
@@ -81,21 +81,30 @@ struct iovec {
#define ETH_P_NCSI(0x88f8)
#define ETH_P_UNKNOWN (0x)
+/* FIXME: remove me when made standalone */
#ifdef _WIN32
+#undef closesocket
+#undef getsockopt
+#undef ioctlsocket
+#undef recv
+#undef setsockopt
+#endif
+
+#ifdef _WIN32
+#define closesocket slirp_closesocket
int slirp_closesocket(int fd);
+#define ioctlsocket slirp_ioctlsocket
int slirp_ioctlsocket(int fd, int req, void *val);
-int inet_aton(const char *cp, struct in_addr *ia);
-#define slirp_getsockopt(sockfd, level, optname, optval, optlen) \
+#define getsockopt(sockfd, level, optname, optval, optlen) \
getsockopt(sockfd, level, optname, (void *)optval, optlen)
-#define slirp_setsockopt(sockfd, level, optname, optval, optlen)\
+#define setsockopt(sockfd, level, optname, optval, optlen)\
setsockopt(sockfd, level, optname, (const void *)optval, optlen)
-#define slirp_recv(sockfd, buf, len, flags) recv(sockfd, (void *)buf, len,
flags)
+#define recv(sockfd, buf, len, flags) recv(sockfd, (void *)buf, len, flags)
+
+int inet_aton(const char *cp, struct in_addr *ia);
#else
-#define slirp_setsockopt setsockopt
-#define slirp_getsockopt getsockopt
-#define slirp_recv recv
-#define slirp_closesocket close
-#define slirp_ioctlsocket ioctl
+#define closesocket(s) close(s)
+#define ioctlsocket(s, r, v) ioctl(s, r, v)
#endif
int slirp_socket(int domain, int type, int protocol);
@@ -104,14 +113,14 @@ void slirp_set_nonblock(int fd);
static inline int slirp_socket_set_nodelay(int fd)
{
int v = 1;
-return slirp_setsockopt(fd, IPPROTO_TCP, TCP_NODELAY, , sizeof(v));
+return setsockopt(fd, IPPROTO_TCP, TCP_NODELAY, , sizeof(v));
}
static inline int slirp_socket_set_fast_reuse(int fd)
{
#ifndef _WIN32
int v = 1;
-return slirp_setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, , sizeof(v));
+return setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, , sizeof(v));
#else
/* Enabling the reuse of an endpoint that was used by a socket still in
* TIME_WAIT state is usually performed by setting SO_REUSEADDR. On Windows
diff --git a/slirp/ip_icmp.c b/slirp/ip_icmp.c
index ce79c0b051..120108f582 100644
--- a/slirp/ip_icmp.c
+++ b/slirp/ip_icmp.c
@@ -114,7 +114,7 @@ static int icmp_send(struct socket *so, struct mbuf *m, int
hlen)
void icmp_detach(struct socket *so)
{
so->slirp->cb->unregister_poll_fd(so->s, so->slirp->opaque);
-slirp_closesocket(so->s);
+closesocket(so->s);
sofree(so);
}
@@ -421,7 +421,7 @@ void icmp_receive(struct socket *so)
icp = mtod(m, struct icmp *);
id = icp->icmp_id;
-len = slirp_recv(so->s, icp, M_ROOM(m), 0);
+len = recv(so->s, icp, M_ROOM(m), 0);
/*
* The behavior of reading SOCK_DGRAM+IPPROTO_ICMP sockets is inconsistent
* between host OSes. On Linux, only the ICMP header and payload is
diff --git a/slirp/misc.c b/slirp/misc.c
index 3f4cd852f8..d9fc586a24 100644
--- a/slirp/misc.c
+++ b/slirp/misc.c
@@ -98,16 +98,16 @@ slirp_socketpair_with_oob(int sv[2])
goto err;
}
-slirp_closesocket(s);
+closesocket(s);
return 0;
err:
g_critical("slirp_socketpair(): %s", strerror(errno));
if (s >= 0) {
-slirp_closesocket(s);
+closesocket(s);
}
if (sv[1] >= 0) {
-slirp_closesocket(sv[1]);
+closesocket(sv[1]);
}
return -1;
}
@@ -211,16 +211,16 @@ fork_exec(struct socket *so, const char *ex)
if (err) {
g_critical("fork_exec: %s", err->message);
g_error_free(err);
-slirp_closesocket(sp[0]);
-slirp_closesocket(sp[1]);
+closesocket(sp[0]);
+closesocket(sp[1]);
return 0;
}
so->s = sp[0];
-slirp_closesocket(sp[1]);
+
[Qemu-devel] [PATCH 0/2] Fix slirp regression on win32
Hi, QEMU wraps the socket functions in os-win32.h, but in commit a9d8b3ec4385793815d71217857304, the header inclusion was dropped, breaking slirp on Windows. Fix the regression by wrapping all the socket functions. thanks Marc-André Lureau (2): slirp: remove slirp_ prefix for socket wrappers slirp: wrap the remaining socket functions slirp/util.h | 75 ++ slirp/ip_icmp.c | 4 +- slirp/misc.c | 14 ++-- slirp/slirp.c| 2 +- slirp/socket.c | 14 ++-- slirp/tcp_subr.c | 10 +-- slirp/udp.c | 2 +- slirp/util.c | 164 ++- 8 files changed, 246 insertions(+), 39 deletions(-) -- 2.21.0.rc0.1.g036caf7885
[Qemu-devel] [PATCH 3/7] slirp: use "slirp_" prefix for inet_aton() win32 implementation
To avoid conflict with QEMU inet_aton() implementation, let's use the
"slirp_" prefix. This allows to drop the WITH_QEMU, thus the source
won't make a distinction when building with QEMU or not.
Signed-off-by: Marc-André Lureau
---
slirp/util.h| 4 ++--
slirp/util.c| 4 ++--
slirp/Makefile.objs | 2 +-
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/slirp/util.h b/slirp/util.h
index c4207a49d6..e94ee4e7f1 100644
--- a/slirp/util.h
+++ b/slirp/util.h
@@ -138,8 +138,8 @@ int slirp_getsockopt_wrap(int sockfd, int level, int
optname,
#define setsockopt slirp_setsockopt_wrap
int slirp_setsockopt_wrap(int sockfd, int level, int optname,
const void *optval, int optlen);
-
-int inet_aton(const char *cp, struct in_addr *ia);
+#define inet_aton slirp_inet_aton
+int slirp_inet_aton(const char *cp, struct in_addr *ia);
#else
#define closesocket(s) close(s)
#define ioctlsocket(s, r, v) ioctl(s, r, v)
diff --git a/slirp/util.c b/slirp/util.c
index 1cbaa26b60..5ec2fa87ab 100644
--- a/slirp/util.c
+++ b/slirp/util.c
@@ -31,8 +31,8 @@
#include
#include
-#if defined(_WIN32) && !defined(WITH_QEMU)
-int inet_aton(const char *cp, struct in_addr *ia)
+#if defined(_WIN32)
+int slirp_inet_aton(const char *cp, struct in_addr *ia)
{
uint32_t addr = inet_addr(cp);
if (addr == 0x) {
diff --git a/slirp/Makefile.objs b/slirp/Makefile.objs
index 69e140f965..e91daf0e91 100644
--- a/slirp/Makefile.objs
+++ b/slirp/Makefile.objs
@@ -33,4 +33,4 @@ slirp.mo-objs = \
vmstate.o \
$(NULL)
-slirp.mo-cflags = -DG_LOG_DOMAIN=\"Slirp\" -DWITH_QEMU
+slirp.mo-cflags = -DG_LOG_DOMAIN=\"Slirp\"
--
2.21.0.rc0.1.g036caf7885
[Qemu-devel] [PATCH 7/7] slirp: remove QEMU Makefile.objs
QEMU no longer includes it, and treats slirp/ as a separate project. Signed-off-by: Marc-André Lureau --- slirp/Makefile.objs | 36 1 file changed, 36 deletions(-) delete mode 100644 slirp/Makefile.objs diff --git a/slirp/Makefile.objs b/slirp/Makefile.objs deleted file mode 100644 index 0250229dfa..00 --- a/slirp/Makefile.objs +++ /dev/null @@ -1,36 +0,0 @@ -slirp-obj-y = slirp.mo - -slirp.mo-objs = \ - src/arp_table.o \ - src/bootp.o \ - src/cksum.o \ - src/dhcpv6.o \ - src/dnssearch.o \ - src/if.o \ - src/ip6_icmp.o \ - src/ip6_input.o \ - src/ip6_output.o \ - src/ip_icmp.o \ - src/ip_input.o \ - src/ip_output.o \ - src/mbuf.o \ - src/misc.o \ - src/ncsi.o \ - src/ndp_table.o \ - src/sbuf.o \ - src/slirp.o \ - src/socket.o \ - src/state.o \ - src/stream.o \ - src/tcp_input.o \ - src/tcp_output.o \ - src/tcp_subr.o \ - src/tcp_timer.o \ - src/tftp.o \ - src/udp.o \ - src/udp6.o \ - src/util.o \ - src/vmstate.o \ - $(NULL) - -slirp.mo-cflags = -DG_LOG_DOMAIN=\"Slirp\" -- 2.21.0.rc0.1.g036caf7885
Re: [Qemu-devel] [RFC PULL 0/6] Block sgarzare test patches
On Tue, 12 Feb 2019 at 14:12, Stefan Hajnoczi wrote: > > Block pull request for testing > > Peter hit a virtio-blk-test failure caused by the new DISCARD/WRITE_ZEROES > patches that Stefano and I have been unable to reproduce. Here are the > patches > so they can be tested again in Peter's environment. > > x86-64 host seems to be a bit random about whether it reproducibly fails (it failed in the test merge builds but not when I run the test program by hand) but aarch64 host seems more definitely fails-every-time: pm215@gcc113:~/qemu/build/all$ QTEST_QEMU_BINARY=ppc64-softmmu/qemu-system-ppc64 strace -tt -f -o ~/strace.log tests/virtio-blk-test /ppc64/virtio/blk/pci/basic: ** ERROR:/home/pm215/qemu/tests/virtio-blk-test.c:272:test_basic: assertion failed (status == 0): (1 == 0) Aborted (core dumped) The strace.log from that is at http://people.linaro.org/~peter.maydell/strace.log (3.6MB file; the -f means it will have the trace for both the test binary and the QEMU process it spawns). thanks -- PMM
Re: [Qemu-devel] [PULL 00/25] pci, pc, virtio: fixes, cleanups, features
On Tue, Feb 12, 2019 at 11:39:21AM +0100, Philippe Mathieu-Daudé wrote: > On 2/12/19 8:11 AM, Peter Xu wrote: > > On Tue, Feb 05, 2019 at 11:06:42AM -0500, Michael S. Tsirkin wrote: > >> > >> OK I reverted the whole part dealing with vhost-user and reposted. > > > > I noticed that the merged pull request could possibly have squashed > > the below two patches (in previous pull) into one super patch > > (a56de056c91f87e1e): > > > > i386/kvm: ignore masked irqs when update msi routes > > contrib/vhost-user-blk: fix the compilation issue > > > > Here, the first patch lost its commit message, and the last patch lost > > its real author. ;) > > I suggest we revert a56de056c9 ASAP and reapply the both patches, this > will ease cherry-picking/downstream workflow. I don't see why does it help upstream. -- MST
Re: [Qemu-devel] [PATCH 10/15] s390-bios: Support for running format-0/1 channel programs
On Tue, 5 Feb 2019 11:18:38 +0100
Cornelia Huck wrote:
> On Mon, 4 Feb 2019 14:29:18 -0500
> Farhan Ali wrote:
>
> > On 02/04/2019 06:13 AM, Cornelia Huck wrote:
> > > On Thu, 31 Jan 2019 12:31:00 -0500
> > > Farhan Ali wrote:
> > >
> > >> On 01/29/2019 08:29 AM, Jason J. Herne wrote:
> > >>> Add struct for format-0 ccws. Support executing format-0 channel
> > >>> programs and waiting for their completion before continuing execution.
> > >>> This will be used for real dasd ipl.
> > >>>
> > >>> Add cu_type() to channel io library. This will be used to query control
> > >>> unit type which is used to determine if we are booting a virtio device
> > >>> or a
> > >>> real dasd device.
> > >>>
> > >>> Signed-off-by: Jason J. Herne
> > >>> ---
> > >>>pc-bios/s390-ccw/cio.c | 114
> > >>> +++
> > >>>pc-bios/s390-ccw/cio.h | 127
> > >>> ++--
> > >>>pc-bios/s390-ccw/s390-ccw.h | 1 +
> > >>>pc-bios/s390-ccw/start.S| 33 +++-
> > >>>4 files changed, 270 insertions(+), 5 deletions(-)
> > >
> > >>> +/*
> > >>> + * Executes a channel program at a given subchannel. The request to
> > >>> run the
> > >>> + * channel program is sent to the subchannel, we then wait for the
> > >>> interrupt
> > >>> + * signaling completion of the I/O operation(s) performed by the
> > >>> channel
> > >>> + * program. Lastly we verify that the i/o operation completed without
> > >>> error and
> > >>> + * that the interrupt we received was for the subchannel used to run
> > >>> the
> > >>> + * channel program.
> > >>> + *
> > >>> + * Note: This function assumes it is running in an environment where
> > >>> no other
> > >>> + * cpus are generating or receiving I/O interrupts. So either run it
> > >>> in a
> > >>> + * single-cpu environment or make sure all other cpus are not doing
> > >>> I/O and
> > >>> + * have I/O interrupts masked off.
> > >>> + */
> > >>> +int do_cio(SubChannelId schid, uint32_t ccw_addr, int fmt)
> > >>> +{
> > >>> +CmdOrb orb = {};
> > >>> +Irb irb = {};
> > >>> +sense_data_eckd_dasd sd;
> > >>> +int rc, retries = 0;
> > >>> +
> > >>> +IPL_assert(fmt == 0 || fmt == 1, "Invalid ccw format");
> > >>> +
> > >>> +/* ccw_addr must be <= 24 bits and point to at least one whole
> > >>> ccw. */
> > >>> +if (fmt == 0) {
> > >>> +IPL_assert(ccw_addr <= 0xFF - 8, "Invalid ccw address");
> > >>> +}
> > >>> +
> > >>> +orb.fmt = fmt ;
> > >>> +orb.pfch = 1; /* QEMU's cio implementation requires prefetch */
> > >>> +orb.c64 = 1; /* QEMU's cio implementation requires 64-bit idaws
> > >>> */
> > >>> +orb.lpm = 0xFF; /* All paths allowed */
> > >>> +orb.cpa = ccw_addr;
> > >>> +
> > >>> +while (true) {
> > >>> +rc = ssch(schid, );
> > >>> +if (rc == 1) {
> > >>> +/* Status pending, not sure why. Let's eat the status and
> > >>> retry. */
> > >>> +tsch(schid, );
> > >>> +retries++;
> > >>> +continue;
> > >>> +}
> > >>> +if (rc) {
> > >>> +print_int("ssch failed with rc=", rc);
> > >>> +break;
> > >>> +}
> > >>> +
> > >>> +consume_io_int();
> > >>> +
> > >>> +/* collect status */
> > >>> +rc = tsch(schid, );
> > >>> +if (rc) {
> > >>> +print_int("tsch failed with rc=", rc);
> > >>> +break;
> > >>> +}
> > >>> +
> > >>> +if (!irb_error()) {
> > >>> +break;
> > >>> +}
> > >>> +
> > >>> +/*
> > >>> + * Unexpected unit check, or interface-control-check. Use
> > >>> sense to
> > >>> + * clear unit check then retry.
> > >>> + */
> > >>> +if ((unit_check() || iface_ctrl_check()) && retries <=
> > >>> 2) {
> > >>> +basic_sense(schid, , sizeof(sd));
> > >>
> > >> We are using basic sense to clear any unit check or ifcc, but is it
> > >> possible for the basic sense to cause another unit check?
> > >>
> > >> The chapter on Basic Sense in the Common I/O Device Commands
> > >> (http://publibz.boulder.ibm.com/support/libraryserver/FRAMESET/dz9ar501/2.1?SHELF==19920409154647=)
> > >>says this:
> > >>
> > >> ""
> > >> The basic sense command initiates a sense operation at all devices
> > >> and cannot cause the command-reject, intervention-required,
> > >> data-check, or overrun bit to be set to one. If the control unit
> > >> detects an equipment malfunction or invalid checking-block code
> > >> (CBC) on the sense-command code, the equipment-check or bus-out-check
> > >> bit is set to one, and unit check is indicated in the device-status
> > >> byte.
> > >> ""
> > >>
> > >> If my understanding is correct, if there is an equipment malfunction
> > >> then the control unit can return a unit check even for a basic sense.
> > >> This can lead to infinite recursion in
Re: [Qemu-devel] Key repeat is no longer working on TTY and grub menu
On Tue, Feb 12, 2019 at 01:02:42PM +, Leonardo Soares Müller wrote: > I noticed that the key inputs are no longer repeating when using default > setting on QEMU. For example, if I press "a" and keep it pressed it will > print only one "a" instead of repeating them. Another example is when > deleting text: instead of holding backspace pressed and the text is > deleted, it's needed to press backspace once for each character. > > As this makes many tasks much slower, requiring many key presses for > tasks that were done in a single press, I did a bisect to see when this > change happened. The result is: > > 0c0d42737dfdcea872a987ecba6ef55047df8881 is the first bad commit > commit 0c0d42737dfdcea872a987ecba6ef55047df8881 > Author: Gerd Hoffmann > Date: Tue Jan 22 10:28:11 2019 +0100 > > kbd-state: use state tracker for gtk > > Use the new keyboard state tracked for gtk. Allows to drop the > gtk-specific modifier state tracking code. > > Signed-off-by: Gerd Hoffmann > Reviewed-by: Daniel P. Berrangé > Message-id: 20190122092814.14919-6-kra...@redhat.com > > :04 04 e882f144bafca90257a4604b5e62486a1ca038b2 > 0139afe1aee24256c3f711c080b0230ab104074a Minclude > :04 04 c258efc06107b944b0627792b9f2377600033118 > 4fd01a25a3e0d1fccca72c35ec58b205aa4fd882 Mui > > I'm using the GTK interface and all guests without a Graphical User > Interface (GUI) are affected by this. This happens on grub menu and on > TTY of multiple Linux guests. Yes, this is another regression accidentally introduced by the keyboard state tracker. When GTK does key repeat it omits the Up event for repeated keys. IOW, you get Press (a) Press (a) Press (a) Release (a) Not Press (a) Release (a) Press (a) Release (a) Press (a) Release (a) The keyboard state tracker doesn't take this into account, so it is surpressing all except the first Press event. This might affect other frontends too if they use the same trick for key repeat Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [Qemu-devel] [PATCH v2] s390x: add zPCI feature to "qemu" CPU model
On Tue, 12 Feb 2019 12:23:23 +0100 David Hildenbrand wrote: > As we now always have PCI support, let's add it to the "qemu" CPU model, > taking care of backwards compatibility. > > Signed-off-by: David Hildenbrand > --- > > v1 -> v2: > - Use correct model identifiction of the z12 we emulate > > hw/s390x/s390-virtio-ccw.c | 3 +++ > target/s390x/gen-features.c | 8 ++-- > 2 files changed, 9 insertions(+), 2 deletions(-) > Played around a bit with different machines and cpu models and got what I expected. Thanks, applied.
Re: [Qemu-devel] [PATCH v2 3/3] tpm_tis: fix format string specifier in tpm_tis_show_buffer()
On 12/02/2019 13:27, Stefan Berger wrote:
On 2/12/19 7:31 AM, Philippe Mathieu-Daudé wrote:
On 2/11/19 10:13 PM, Stefan Berger wrote:
On 2/11/19 3:09 PM, Liam Merwick wrote:
On 11/02/2019 19:56, Stefan Berger wrote:
On 2/11/19 11:02 AM, Philippe Mathieu-Daudé wrote:
On 2/11/19 4:03 PM, Liam Merwick wrote:
[...]
- printf("tpm_tis: %s length = %d\n", string, len);
+ printf("tpm_tis: %s length = %u\n", string, len);
So here the format is '%zu'.
However in code cleanup we try go get ride of printf() calls and
replace them with trace points.
This code is only used for debugging if DEBUG_TIS has been #defined.
No need to add tracing here.
I'd come up the attached change (but that seems like overkill).
I don't think we need tracing for this.
So if you think the code is mature enough, let's remove the DEBUG calls!
Else we prefer to convert DEBUG printf to trace events because (at
least):
- no need to recompile to enable debugging
- when compiled with debugging, you don't mess with STDIO which can be
used as a chardev backend.
Fine. Then I withdraw my reviewed-by.
I don't see a way of removing the DEBUG calls without adding the
overhead of a call to tpm_tis_show_buffer() each time even if tracing is
not enabled (the 3 trace calls are interdependent and need a for loop).
Is there any example of a trace point that calls a function that then
does non-trivial printing?
I could send a v3 with the patch I attached previously with the 3 printf
calls changed to trace points but still wrapped in 'if (DEBUG_TIS)' and
optimised out in non-debug.
Regards,
Liam
[Qemu-devel] [PATCH 3/3] i386/kvm: ignore masked irqs when update msi routes
From: Peter Xu
When we are with intel-iommu device and with IR on, KVM will register
an IEC notifier to detect interrupt updates from the guest and we'll
kick off kvm_update_msi_routes_all() when it happens to make sure
kernel IRQ cache is matching the latest.
Though, kvm_update_msi_routes_all() is buggy in that it ignored the
mask bit of either MSI/MSIX messages and it tries to translate the
message even if the corresponding message was already masked by the
guest driver (hence the MSI/MSIX message will be invalid).
Without this patch, we can receive an error message when we reboot a
guest with both an assigned vfio-pci device and intel-iommu enabled:
qemu-system-x86_64: vtd_interrupt_remap_msi: MSI address low 32 bit invalid:
0x0
The error does not affect functionality of the guest since when we
failed to translate we'll just silently continue (which makes sense
since crashing the VM for this seems even worse), but still it's
better to fix it up.
Signed-off-by: Peter Xu
Reviewed-by: Michael S. Tsirkin
Message-Id: <20190116030815.27273-5-pet...@redhat.com>
[PMD: this patch was first (incorrectly) introduced as a56de056c91f8]
Signed-off-by: Philippe Mathieu-Daudé
---
target/i386/kvm.c | 14 +++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 9af4542fb8..beae1b99da 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -3894,7 +3894,7 @@ static QLIST_HEAD(, MSIRouteEntry) msi_route_list = \
static void kvm_update_msi_routes_all(void *private, bool global,
uint32_t index, uint32_t mask)
{
-int cnt = 0;
+int cnt = 0, vector;
MSIRouteEntry *entry;
MSIMessage msg;
PCIDevice *dev;
@@ -3902,11 +3902,19 @@ static void kvm_update_msi_routes_all(void *private,
bool global,
/* TODO: explicit route update */
QLIST_FOREACH(entry, _route_list, list) {
cnt++;
+vector = entry->vector;
dev = entry->dev;
-if (!msix_enabled(dev) && !msi_enabled(dev)) {
+if (msix_enabled(dev) && !msix_is_masked(dev, vector)) {
+msg = msix_get_message(dev, vector);
+} else if (msi_enabled(dev) && !msi_is_masked(dev, vector)) {
+msg = msi_get_message(dev, vector);
+} else {
+/*
+ * Either MSI/MSIX is disabled for the device, or the
+ * specific message was masked out. Skip this one.
+ */
continue;
}
-msg = pci_get_msi_message(dev, entry->vector);
kvm_irqchip_update_msi_route(kvm_state, entry->virq, msg, dev);
}
kvm_irqchip_commit_routes(kvm_state);
--
2.20.1
[Qemu-devel] [PATCH] chardev/wctablet: Fix a typo
The correct name is Wacom.
Fix the typo present this the origin of this file (378af96155d).
Signed-off-by: Philippe Mathieu-Daudé
---
chardev/wctablet.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/chardev/wctablet.c b/chardev/wctablet.c
index 969d014574..35dbd29a33 100644
--- a/chardev/wctablet.c
+++ b/chardev/wctablet.c
@@ -177,7 +177,7 @@ static void wctablet_input_sync(DeviceState *dev)
}
static QemuInputHandler wctablet_handler = {
-.name = "QEMU Wacome Pen Tablet",
+.name = "QEMU Wacom Pen Tablet",
.mask = INPUT_EVENT_MASK_BTN | INPUT_EVENT_MASK_ABS,
.event = wctablet_input_event,
.sync = wctablet_input_sync,
--
2.20.1
[Qemu-devel] [PATCH] hw/dma/i8257: Use qemu_log_mask(UNIMP) instead of fprintf
Avoid to clutter stdout until explicitly requested (with -d unimp):
$ qemu-system-mips64el -M fulong2e -bios pmon_2e.bin
dma: command df not supported
dma: command df not supported
dma: command df not supported
dma: command df not supported
Signed-off-by: Philippe Mathieu-Daudé
---
hw/dma/i8257.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/hw/dma/i8257.c b/hw/dma/i8257.c
index 52675e97c9..3e1f13a4aa 100644
--- a/hw/dma/i8257.c
+++ b/hw/dma/i8257.c
@@ -26,6 +26,7 @@
#include "hw/isa/isa.h"
#include "hw/dma/i8257.h"
#include "qemu/main-loop.h"
+#include "qemu/log.h"
#include "trace.h"
#define I8257(obj) \
@@ -185,7 +186,8 @@ static void i8257_write_cont(void *opaque, hwaddr nport,
uint64_t data,
switch (iport) {
case 0x00: /* command */
if ((data != 0) && (data & CMD_NOT_SUPPORTED)) {
-dolog("command %"PRIx64" not supported\n", data);
+qemu_log_mask(LOG_UNIMP, "%s: cmd 0x%02"PRIx64" not supported\n",
+ __func__, data);
return;
}
d->command = data;
--
2.20.1
[Qemu-devel] [PATCH 4/7] slirp: move sources to src/ subdirectory
Prepare for making slirp/ a standalone project.
Remove some useless includes while at it.
Signed-off-by: Marc-André Lureau
---
slirp/{ => src}/bootp.h | 0
slirp/{ => src}/debug.h | 0
slirp/{ => src}/dhcpv6.h | 0
slirp/{ => src}/if.h | 0
slirp/{ => src}/ip.h | 0
slirp/{ => src}/ip6.h| 0
slirp/{ => src}/ip6_icmp.h | 0
slirp/{ => src}/ip_icmp.h| 0
slirp/{ => src}/libslirp.h | 0
slirp/{ => src}/main.h | 0
slirp/{ => src}/mbuf.h | 0
slirp/{ => src}/misc.h | 0
slirp/{ => src}/ncsi-pkt.h | 0
slirp/{ => src}/qtailq.h | 0
slirp/{ => src}/sbuf.h | 0
slirp/{ => src}/slirp.h | 0
slirp/{ => src}/socket.h | 0
slirp/{ => src}/state.h | 0
slirp/{ => src}/stream.h | 0
slirp/{ => src}/tcp.h| 0
slirp/{ => src}/tcp_timer.h | 0
slirp/{ => src}/tcp_var.h| 0
slirp/{ => src}/tcpip.h | 0
slirp/{ => src}/tftp.h | 0
slirp/{ => src}/udp.h| 0
slirp/{ => src}/util.h | 0
slirp/{ => src}/vmstate.h| 0
net/slirp.c | 2 +-
slirp/{ => src}/arp_table.c | 0
slirp/{ => src}/bootp.c | 0
slirp/{ => src}/cksum.c | 0
slirp/{ => src}/dhcpv6.c | 0
slirp/{ => src}/dnssearch.c | 0
slirp/{ => src}/if.c | 0
slirp/{ => src}/ip6_icmp.c | 0
slirp/{ => src}/ip6_input.c | 0
slirp/{ => src}/ip6_output.c | 0
slirp/{ => src}/ip_icmp.c| 0
slirp/{ => src}/ip_input.c | 0
slirp/{ => src}/ip_output.c | 0
slirp/{ => src}/mbuf.c | 0
slirp/{ => src}/misc.c | 0
slirp/{ => src}/ncsi.c | 0
slirp/{ => src}/ndp_table.c | 0
slirp/{ => src}/sbuf.c | 0
slirp/{ => src}/slirp.c | 0
slirp/{ => src}/socket.c | 0
slirp/{ => src}/state.c | 0
slirp/{ => src}/stream.c | 0
slirp/{ => src}/tcp_input.c | 0
slirp/{ => src}/tcp_output.c | 0
slirp/{ => src}/tcp_subr.c | 0
slirp/{ => src}/tcp_timer.c | 0
slirp/{ => src}/tftp.c | 0
slirp/{ => src}/udp.c| 0
slirp/{ => src}/udp6.c | 0
slirp/{ => src}/util.c | 0
slirp/{ => src}/vmstate.c| 0
util/main-loop.c | 2 --
vl.c | 3 --
slirp/Makefile.objs | 60 ++--
61 files changed, 31 insertions(+), 36 deletions(-)
rename slirp/{ => src}/bootp.h (100%)
rename slirp/{ => src}/debug.h (100%)
rename slirp/{ => src}/dhcpv6.h (100%)
rename slirp/{ => src}/if.h (100%)
rename slirp/{ => src}/ip.h (100%)
rename slirp/{ => src}/ip6.h (100%)
rename slirp/{ => src}/ip6_icmp.h (100%)
rename slirp/{ => src}/ip_icmp.h (100%)
rename slirp/{ => src}/libslirp.h (100%)
rename slirp/{ => src}/main.h (100%)
rename slirp/{ => src}/mbuf.h (100%)
rename slirp/{ => src}/misc.h (100%)
rename slirp/{ => src}/ncsi-pkt.h (100%)
rename slirp/{ => src}/qtailq.h (100%)
rename slirp/{ => src}/sbuf.h (100%)
rename slirp/{ => src}/slirp.h (100%)
rename slirp/{ => src}/socket.h (100%)
rename slirp/{ => src}/state.h (100%)
rename slirp/{ => src}/stream.h (100%)
rename slirp/{ => src}/tcp.h (100%)
rename slirp/{ => src}/tcp_timer.h (100%)
rename slirp/{ => src}/tcp_var.h (100%)
rename slirp/{ => src}/tcpip.h (100%)
rename slirp/{ => src}/tftp.h (100%)
rename slirp/{ => src}/udp.h (100%)
rename slirp/{ => src}/util.h (100%)
rename slirp/{ => src}/vmstate.h (100%)
rename slirp/{ => src}/arp_table.c (100%)
rename slirp/{ => src}/bootp.c (100%)
rename slirp/{ => src}/cksum.c (100%)
rename slirp/{ => src}/dhcpv6.c (100%)
rename slirp/{ => src}/dnssearch.c (100%)
rename slirp/{ => src}/if.c (100%)
rename slirp/{ => src}/ip6_icmp.c (100%)
rename slirp/{ => src}/ip6_input.c (100%)
rename slirp/{ => src}/ip6_output.c (100%)
rename slirp/{ => src}/ip_icmp.c (100%)
rename slirp/{ => src}/ip_input.c (100%)
rename slirp/{ => src}/ip_output.c (100%)
rename slirp/{ => src}/mbuf.c (100%)
rename slirp/{ => src}/misc.c (100%)
rename slirp/{ => src}/ncsi.c (100%)
rename slirp/{ => src}/ndp_table.c (100%)
rename slirp/{ => src}/sbuf.c (100%)
rename slirp/{ => src}/slirp.c (100%)
rename slirp/{ => src}/socket.c (100%)
rename slirp/{ => src}/state.c (100%)
rename slirp/{ => src}/stream.c (100%)
rename slirp/{ => src}/tcp_input.c (100%)
rename slirp/{ => src}/tcp_output.c (100%)
rename slirp/{ => src}/tcp_subr.c (100%)
rename slirp/{ => src}/tcp_timer.c (100%)
rename slirp/{ => src}/tftp.c (100%)
rename slirp/{ => src}/udp.c (100%)
rename slirp/{ => src}/udp6.c (100%)
rename slirp/{ => src}/util.c (100%)
rename slirp/{ => src}/vmstate.c (100%)
diff --git a/slirp/bootp.h b/slirp/src/bootp.h
similarity index 100%
rename from slirp/bootp.h
rename to slirp/src/bootp.h
diff --git a/slirp/debug.h b/slirp/src/debug.h
similarity index 100%
rename from slirp/debug.h
rename to slirp/src/debug.h
diff --git a/slirp/dhcpv6.h b/slirp/src/dhcpv6.h
similarity index 100%
rename from slirp/dhcpv6.h
rename to
Re: [Qemu-devel] [PATCH 07/13] block: Allow omitting the 'backing' option in certain cases
Am 17.01.2019 um 16:33 hat Alberto Garcia geschrieben:
> Of all options of type BlockdevRef used to specify children in
> BlockdevOptions, 'backing' is the only one that is optional.
>
> For "x-blockdev-reopen" we want that if an option is omitted then it
> must be reset to its default value. The default value of 'backing'
> means that QEMU opens the backing file specified in the image
> metadata, but this is not something that we want to support for the
> reopen operation.
>
> Because of this the 'backing' option has to be specified during
> reopen, pointing to the existing backing file if we want to keep it,
> or pointing to a different one (or NULL) if we want to replace it (to
> be implemented in a subsequent patch).
>
> In order to simplify things a bit and not to require that the user
> passes the 'backing' option to every single block device even when
> it's clearly not necessary, this patch allows omitting this option if
> the block device being reopened doesn't have a backing file attached
> _and_ no default backing file is specified in the image metadata.
>
> Signed-off-by: Alberto Garcia
> ---
> block.c | 8 +++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/block.c b/block.c
> index fd51f1cd35..897c8b85cd 100644
> --- a/block.c
> +++ b/block.c
> @@ -3336,7 +3336,13 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state,
> BlockReopenQueue *queue,
>
> drv_prepared = true;
>
> -if (reopen_state->backing_missing) {
> +/*
> + * We must provide the 'backing' option if the BDS has a backing
> + * file or if the image file has a backing file name as part of
> + * its metadata. Otherwise the 'backing' option can be omitted.
> + */
> +if (reopen_state->backing_missing &&
> +(backing_bs(reopen_state->bs) || reopen_state->bs->backing_file[0]))
> {
> error_setg(errp, "backing is missing for '%s'",
> reopen_state->bs->node_name);
> ret = -EINVAL;
Okay, this should be enough to fix drivers without support for backing
files again. Might be worth mentioning in the commit message of this and
the previous patch.
Normally, I would suggest merging this into the previous patch to keep
things bisectable, but keep_old_opts == false isn't used yet, so this
isn't a concern in this case.
Kevin
Re: [Qemu-devel] [PATCH v1 4/9] {monitor, hw/pvrdma}: Expose device internals via monitor interface
On Sun, Feb 10, 2019 at 12:45:32PM +0200, Yuval Shaia wrote:
> Allow interrogating device internals through HMP interface.
> The exposed indicators can be used for troubleshooting by developers or
> sysadmin.
> There is no need to expose these attributes to a management system (e.x.
> libvirt) because (1) most of them are not "device-management' related
> info and (2) there is no guarantee the interface is stable.
Hi David,
So is is a ack or nack?
Appreciate,
Thanks,
Yuval
>
> Signed-off-by: Yuval Shaia
> ---
> hmp-commands-info.hx | 16
> hw/rdma/rdma_backend.c| 70 ++-
> hw/rdma/rdma_rm.c | 7
> hw/rdma/rdma_rm_defs.h| 27 +-
> hw/rdma/vmw/pvrdma.h | 5 +++
> hw/rdma/vmw/pvrdma_hmp.h | 21 +++
> hw/rdma/vmw/pvrdma_main.c | 77 +++
> monitor.c | 10 +
> 8 files changed, 215 insertions(+), 18 deletions(-)
> create mode 100644 hw/rdma/vmw/pvrdma_hmp.h
>
> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> index cbee8b944d..9153c33974 100644
> --- a/hmp-commands-info.hx
> +++ b/hmp-commands-info.hx
> @@ -524,6 +524,22 @@ STEXI
> Show CPU statistics.
> ETEXI
>
> +#if defined(CONFIG_PVRDMA)
> +{
> +.name = "pvrdmacounters",
> +.args_type = "",
> +.params = "",
> +.help = "show pvrdma device counters",
> +.cmd= hmp_info_pvrdmacounters,
> +},
> +
> +STEXI
> +@item info pvrdmacounters
> +@findex info pvrdmacounters
> +Show pvrdma device counters.
> +ETEXI
> +#endif
> +
> #if defined(CONFIG_SLIRP)
> {
> .name = "usernet",
> diff --git a/hw/rdma/rdma_backend.c b/hw/rdma/rdma_backend.c
> index dc2a078b23..b2730f6009 100644
> --- a/hw/rdma/rdma_backend.c
> +++ b/hw/rdma/rdma_backend.c
> @@ -64,9 +64,9 @@ static inline void complete_work(enum ibv_wc_status status,
> uint32_t vendor_err,
> comp_handler(ctx, );
> }
>
> -static void rdma_poll_cq(RdmaDeviceResources *rdma_dev_res, struct ibv_cq
> *ibcq)
> +static int rdma_poll_cq(RdmaDeviceResources *rdma_dev_res, struct ibv_cq
> *ibcq)
> {
> -int i, ne;
> +int i, ne, total_ne = 0;
> BackendCtx *bctx;
> struct ibv_wc wc[2];
>
> @@ -89,12 +89,18 @@ static void rdma_poll_cq(RdmaDeviceResources
> *rdma_dev_res, struct ibv_cq *ibcq)
> rdma_rm_dealloc_cqe_ctx(rdma_dev_res, wc[i].wr_id);
> g_free(bctx);
> }
> +total_ne += ne;
> } while (ne > 0);
> +atomic_sub(_dev_res->stats.missing_cqe, total_ne);
> qemu_mutex_unlock(_dev_res->lock);
>
> if (ne < 0) {
> rdma_error_report("ibv_poll_cq fail, rc=%d, errno=%d", ne, errno);
> }
> +
> +rdma_dev_res->stats.completions += total_ne;
> +
> +return total_ne;
> }
>
> static void *comp_handler_thread(void *arg)
> @@ -122,6 +128,9 @@ static void *comp_handler_thread(void *arg)
> while (backend_dev->comp_thread.run) {
> do {
> rc = qemu_poll_ns(pfds, 1, THR_POLL_TO * (int64_t)SCALE_MS);
> +if (!rc) {
> +backend_dev->rdma_dev_res->stats.poll_cq_ppoll_to++;
> +}
> } while (!rc && backend_dev->comp_thread.run);
>
> if (backend_dev->comp_thread.run) {
> @@ -138,6 +147,7 @@ static void *comp_handler_thread(void *arg)
>errno);
> }
>
> +backend_dev->rdma_dev_res->stats.poll_cq_from_bk++;
> rdma_poll_cq(backend_dev->rdma_dev_res, ev_cq);
>
> ibv_ack_cq_events(ev_cq, 1);
> @@ -271,7 +281,13 @@ int rdma_backend_query_port(RdmaBackendDev *backend_dev,
>
> void rdma_backend_poll_cq(RdmaDeviceResources *rdma_dev_res, RdmaBackendCQ
> *cq)
> {
> -rdma_poll_cq(rdma_dev_res, cq->ibcq);
> +int polled;
> +
> +rdma_dev_res->stats.poll_cq_from_guest++;
> +polled = rdma_poll_cq(rdma_dev_res, cq->ibcq);
> +if (!polled) {
> +rdma_dev_res->stats.poll_cq_from_guest_empty++;
> +}
> }
>
> static GHashTable *ah_hash;
> @@ -333,7 +349,7 @@ static void ah_cache_init(void)
>
> static int build_host_sge_array(RdmaDeviceResources *rdma_dev_res,
> struct ibv_sge *dsge, struct ibv_sge *ssge,
> -uint8_t num_sge)
> +uint8_t num_sge, uint64_t *total_length)
> {
> RdmaRmMR *mr;
> int ssge_idx;
> @@ -349,6 +365,8 @@ static int build_host_sge_array(RdmaDeviceResources
> *rdma_dev_res,
> dsge->length = ssge[ssge_idx].length;
> dsge->lkey = rdma_backend_mr_lkey(>backend_mr);
>
> +*total_length += dsge->length;
> +
> dsge++;
> }
>
> @@ -445,8 +463,10 @@ void rdma_backend_post_send(RdmaBackendDev *backend_dev,
> rc = mad_send(backend_dev, sgid_idx, sgid, sge, num_sge);
> if (rc) {
>
Re: [Qemu-devel] [PATCH 0/2] Fix slirp regression on win32
On Tue, Feb 12, 2019 at 5:09 PM Marc-André Lureau < marcandre.lur...@redhat.com> wrote: > Hi, > > QEMU wraps the socket functions in os-win32.h, but in commit > a9d8b3ec4385793815d71217857304, the header inclusion was dropped, > breaking slirp on Windows. Fix the regression by wrapping all the > socket functions. > > thanks > > Marc-André Lureau (2): > slirp: remove slirp_ prefix for socket wrappers > slirp: wrap the remaining socket functions > > slirp/util.h | 75 ++ > slirp/ip_icmp.c | 4 +- > slirp/misc.c | 14 ++-- > slirp/slirp.c| 2 +- > slirp/socket.c | 14 ++-- > slirp/tcp_subr.c | 10 +-- > slirp/udp.c | 2 +- > slirp/util.c | 164 ++- > 8 files changed, 246 insertions(+), 39 deletions(-) > > -- > 2.21.0.rc0.1.g036caf7885 > Confirmed, slirp working again in qemu-system-ppc for windows, so Tested-by: Howard Spoelstra
Re: [Qemu-devel] [PATCH] hw/arm/armsse: Fix miswiring of expansion IRQs
On 2/12/19 12:06 PM, Peter Maydell wrote:
> On Tue, 12 Feb 2019 at 11:05, Philippe Mathieu-Daudé
> wrote:
>> On 2/12/19 11:52 AM, Peter Maydell wrote:
>>> In commit 91c1e9fcbd7548db368 where we added dual-CPU support to
>>> the ARMSSE, we set up the wiring of the expansion IRQs via nested
>>> loops: the outer loop on 'i' loops for each CPU, and the inner loop
>>> on 'j' loops for each interrupt. Fix a typo which meant we were
>>> wiring every expansion IRQ line to external IRQ 0 on CPU 0 and
>>> to external IRQ 1 on CPU 1.
>>>
>>> Fixes: 91c1e9fcbd7548db368 ("hw/arm/armsse: Support dual-CPU configuration")
>>>
>>> Signed-off-by: Peter Maydell
>>
>> Reviewed-by: Philippe Mathieu-Daudé
>>
>>> ---
>>> It turns out that the ARM-TFM image I was using to test that
>>> I hadn't broken the mps2-an505 doesn't actually rely on any
>>> interrupts from the external devices...
>>
>> How did you notice that, simply reviewing? Via 'info qtree'?
>
> I'm working on a model of a different board (Musca) which also
> uses the SSE-200, and the test code I had for that does happen
> to care about the interrupts.
OK, welcome Musca!
If you are testing Open Source code, please add a few notes on the wiki
or in a mail (or the cover when you introduce the Musca), so we can add
an acceptance test.
Thanks,
Phil.
Re: [Qemu-devel] [PATCH v2 3/3] tpm_tis: fix format string specifier in tpm_tis_show_buffer()
On 2/12/19 7:31 AM, Philippe Mathieu-Daudé wrote:
On 2/11/19 10:13 PM, Stefan Berger wrote:
On 2/11/19 3:09 PM, Liam Merwick wrote:
On 11/02/2019 19:56, Stefan Berger wrote:
On 2/11/19 11:02 AM, Philippe Mathieu-Daudé wrote:
On 2/11/19 4:03 PM, Liam Merwick wrote:
[...]
-printf("tpm_tis: %s length = %d\n", string, len);
+printf("tpm_tis: %s length = %u\n", string, len);
So here the format is '%zu'.
However in code cleanup we try go get ride of printf() calls and
replace them with trace points.
This code is only used for debugging if DEBUG_TIS has been #defined.
No need to add tracing here.
I'd come up the attached change (but that seems like overkill).
I don't think we need tracing for this.
So if you think the code is mature enough, let's remove the DEBUG calls!
Else we prefer to convert DEBUG printf to trace events because (at least):
- no need to recompile to enable debugging
- when compiled with debugging, you don't mess with STDIO which can be
used as a chardev backend.
Fine. Then I withdraw my reviewed-by.
Thanks,
Phil.
Re: [Qemu-devel] [PULL 00/25] pci, pc, virtio: fixes, cleanups, features
On 2/12/19 2:24 PM, Michael S. Tsirkin wrote: > On Tue, Feb 12, 2019 at 02:15:36PM +0100, Philippe Mathieu-Daudé wrote: >> On 2/12/19 2:04 PM, Michael S. Tsirkin wrote: >>> On Tue, Feb 12, 2019 at 11:39:21AM +0100, Philippe Mathieu-Daudé wrote: On 2/12/19 8:11 AM, Peter Xu wrote: > On Tue, Feb 05, 2019 at 11:06:42AM -0500, Michael S. Tsirkin wrote: >> >> OK I reverted the whole part dealing with vhost-user and reposted. > > I noticed that the merged pull request could possibly have squashed > the below two patches (in previous pull) into one super patch > (a56de056c91f87e1e): > > i386/kvm: ignore masked irqs when update msi routes > contrib/vhost-user-blk: fix the compilation issue > > Here, the first patch lost its commit message, and the last patch lost > its real author. ;) I suggest we revert a56de056c9 ASAP and reapply the both patches, this will ease cherry-picking/downstream workflow. >>> >>> I don't see why does it help upstream. >> >> I'd have suggested the same if I had no idea what 'downstream workflow' >> mean, simply to keep the tree clear and avoid to have unrelated changes >> squashed altogether. >> Commit a56de056c9 really looks messy. MSI/MSIX changes described by "fix >> vhost-user-blk compilation". >> Hopefully it won't trigger any problem which requires bisecting to it, >> then contact Changpeng Liu asking him what he intented to do with his >> commit. >> Your call anyway :) >> >> Regards, >> >> Phil. > > > OK these are good points. I'm not sure what happened but it looks like I > screwed up when resolving some conflicts. Care posting a patchset > looking sane? Yes, will do.
[Qemu-devel] [RFC PULL 4/6] virtio-blk: add DISCARD and WRITE_ZEROES features
From: Stefano Garzarella
This patch adds the support of DISCARD and WRITE_ZEROES commands,
that have been introduced in the virtio-blk protocol to have
better performance when using SSD backend.
We support only one segment per request since multiple segments
are not widely used and there are no userspace APIs that allow
applications to submit multiple segments in a single call.
Reviewed-by: Michael S. Tsirkin
Reviewed-by: Stefan Hajnoczi
Signed-off-by: Stefano Garzarella
Acked-by: Pankaj Gupta
Message-id: 20190208134950.187665-5-sgarz...@redhat.com
Signed-off-by: Stefan Hajnoczi
---
include/hw/virtio/virtio-blk.h | 2 +
hw/block/virtio-blk.c | 184 +
2 files changed, 186 insertions(+)
diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h
index f7345b0511..015b523fe0 100644
--- a/include/hw/virtio/virtio-blk.h
+++ b/include/hw/virtio/virtio-blk.h
@@ -38,6 +38,8 @@ struct VirtIOBlkConf
uint32_t request_merging;
uint16_t num_queues;
uint16_t queue_size;
+uint32_t max_discard_sectors;
+uint32_t max_write_zeroes_sectors;
};
struct VirtIOBlockDataPlane;
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 6f2e86264d..5d1b823b66 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -147,6 +147,30 @@ out:
aio_context_release(blk_get_aio_context(s->conf.conf.blk));
}
+static void virtio_blk_discard_write_zeroes_complete(void *opaque, int ret)
+{
+VirtIOBlockReq *req = opaque;
+VirtIOBlock *s = req->dev;
+bool is_write_zeroes = (virtio_ldl_p(VIRTIO_DEVICE(s), >out.type) &
+~VIRTIO_BLK_T_BARRIER) ==
VIRTIO_BLK_T_WRITE_ZEROES;
+
+aio_context_acquire(blk_get_aio_context(s->conf.conf.blk));
+if (ret) {
+if (virtio_blk_handle_rw_error(req, -ret, false, is_write_zeroes)) {
+goto out;
+}
+}
+
+virtio_blk_req_complete(req, VIRTIO_BLK_S_OK);
+if (is_write_zeroes) {
+block_acct_done(blk_get_stats(s->blk), >acct);
+}
+virtio_blk_free_request(req);
+
+out:
+aio_context_release(blk_get_aio_context(s->conf.conf.blk));
+}
+
#ifdef __linux__
typedef struct {
@@ -480,6 +504,84 @@ static bool virtio_blk_sect_range_ok(VirtIOBlock *dev,
return true;
}
+static uint8_t virtio_blk_handle_discard_write_zeroes(VirtIOBlockReq *req,
+struct virtio_blk_discard_write_zeroes *dwz_hdr, bool is_write_zeroes)
+{
+VirtIOBlock *s = req->dev;
+VirtIODevice *vdev = VIRTIO_DEVICE(s);
+uint64_t sector;
+uint32_t num_sectors, flags, max_sectors;
+uint8_t err_status;
+int bytes;
+
+sector = virtio_ldq_p(vdev, _hdr->sector);
+num_sectors = virtio_ldl_p(vdev, _hdr->num_sectors);
+flags = virtio_ldl_p(vdev, _hdr->flags);
+max_sectors = is_write_zeroes ? s->conf.max_write_zeroes_sectors :
+ s->conf.max_discard_sectors;
+
+/*
+ * max_sectors is at most BDRV_REQUEST_MAX_SECTORS, this check
+ * make us sure that "num_sectors << BDRV_SECTOR_BITS" can fit in
+ * the integer variable.
+ */
+if (unlikely(num_sectors > max_sectors)) {
+err_status = VIRTIO_BLK_S_IOERR;
+goto err;
+}
+
+bytes = num_sectors << BDRV_SECTOR_BITS;
+
+if (unlikely(!virtio_blk_sect_range_ok(s, sector, bytes))) {
+err_status = VIRTIO_BLK_S_IOERR;
+goto err;
+}
+
+/*
+ * The device MUST set the status byte to VIRTIO_BLK_S_UNSUPP for discard
+ * and write zeroes commands if any unknown flag is set.
+ */
+if (unlikely(flags & ~VIRTIO_BLK_WRITE_ZEROES_FLAG_UNMAP)) {
+err_status = VIRTIO_BLK_S_UNSUPP;
+goto err;
+}
+
+if (is_write_zeroes) { /* VIRTIO_BLK_T_WRITE_ZEROES */
+int blk_aio_flags = 0;
+
+if (flags & VIRTIO_BLK_WRITE_ZEROES_FLAG_UNMAP) {
+blk_aio_flags |= BDRV_REQ_MAY_UNMAP;
+}
+
+block_acct_start(blk_get_stats(s->blk), >acct, bytes,
+ BLOCK_ACCT_WRITE);
+
+blk_aio_pwrite_zeroes(s->blk, sector << BDRV_SECTOR_BITS,
+ bytes, blk_aio_flags,
+ virtio_blk_discard_write_zeroes_complete, req);
+} else { /* VIRTIO_BLK_T_DISCARD */
+/*
+ * The device MUST set the status byte to VIRTIO_BLK_S_UNSUPP for
+ * discard commands if the unmap flag is set.
+ */
+if (unlikely(flags & VIRTIO_BLK_WRITE_ZEROES_FLAG_UNMAP)) {
+err_status = VIRTIO_BLK_S_UNSUPP;
+goto err;
+}
+
+blk_aio_pdiscard(s->blk, sector << BDRV_SECTOR_BITS, bytes,
+ virtio_blk_discard_write_zeroes_complete, req);
+}
+
+return VIRTIO_BLK_S_OK;
+
+err:
+if (is_write_zeroes) {
+block_acct_invalid(blk_get_stats(s->blk), BLOCK_ACCT_WRITE);
+}
+return err_status;
+}
+
static int virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb)
{
[Qemu-devel] [RFC PULL 6/6] tests/virtio-blk: add test for WRITE_ZEROES command
From: Stefano Garzarella
If the WRITE_ZEROES feature is enabled, we check this command
in the test_basic().
Reviewed-by: Michael S. Tsirkin
Reviewed-by: Stefan Hajnoczi
Acked-by: Thomas Huth
Signed-off-by: Stefano Garzarella
Acked-by: Pankaj Gupta
Message-id: 20190208134950.187665-7-sgarz...@redhat.com
Signed-off-by: Stefan Hajnoczi
---
tests/virtio-blk-test.c | 60 +
1 file changed, 60 insertions(+)
diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
index 0739498da7..35bd92dbfc 100644
--- a/tests/virtio-blk-test.c
+++ b/tests/virtio-blk-test.c
@@ -244,6 +244,66 @@ static void test_basic(QVirtioDevice *dev, QGuestAllocator
*alloc,
guest_free(alloc, req_addr);
+if (features & (1u << VIRTIO_BLK_F_WRITE_ZEROES)) {
+struct virtio_blk_discard_write_zeroes dwz_hdr;
+void *expected;
+
+/*
+ * WRITE_ZEROES request on the same sector of previous test where
+ * we wrote "TEST".
+ */
+req.type = VIRTIO_BLK_T_WRITE_ZEROES;
+req.data = (char *) _hdr;
+dwz_hdr.sector = 0;
+dwz_hdr.num_sectors = 1;
+dwz_hdr.flags = 0;
+
+req_addr = virtio_blk_request(alloc, dev, , sizeof(dwz_hdr));
+
+free_head = qvirtqueue_add(vq, req_addr, 16, false, true);
+qvirtqueue_add(vq, req_addr + 16, sizeof(dwz_hdr), false, true);
+qvirtqueue_add(vq, req_addr + 16 + sizeof(dwz_hdr), 1, true, false);
+
+qvirtqueue_kick(dev, vq, free_head);
+
+qvirtio_wait_used_elem(dev, vq, free_head, NULL,
+ QVIRTIO_BLK_TIMEOUT_US);
+status = readb(req_addr + 16 + sizeof(dwz_hdr));
+g_assert_cmpint(status, ==, 0);
+
+guest_free(alloc, req_addr);
+
+/* Read request to check if the sector contains all zeroes */
+req.type = VIRTIO_BLK_T_IN;
+req.ioprio = 1;
+req.sector = 0;
+req.data = g_malloc0(512);
+
+req_addr = virtio_blk_request(alloc, dev, , 512);
+
+g_free(req.data);
+
+free_head = qvirtqueue_add(vq, req_addr, 16, false, true);
+qvirtqueue_add(vq, req_addr + 16, 512, true, true);
+qvirtqueue_add(vq, req_addr + 528, 1, true, false);
+
+qvirtqueue_kick(dev, vq, free_head);
+
+qvirtio_wait_used_elem(dev, vq, free_head, NULL,
+ QVIRTIO_BLK_TIMEOUT_US);
+status = readb(req_addr + 528);
+g_assert_cmpint(status, ==, 0);
+
+data = g_malloc(512);
+expected = g_malloc0(512);
+memread(req_addr + 16, data, 512);
+g_assert_cmpmem(data, 512, expected, 512);
+g_free(expected);
+g_free(data);
+
+guest_free(alloc, req_addr);
+}
+
if (features & (1u << VIRTIO_F_ANY_LAYOUT)) {
/* Write and read with 2 descriptor layout */
/* Write request */
--
2.20.1
[Qemu-devel] [RFC PULL 1/6] virtio-blk: add acct_failed param to virtio_blk_handle_rw_error()
From: Stefano Garzarella
We add acct_failed param in order to use virtio_blk_handle_rw_error()
also when is not required to call block_acct_failed(). (eg. a discard
operation is failed)
Reviewed-by: Michael S. Tsirkin
Reviewed-by: Stefan Hajnoczi
Signed-off-by: Stefano Garzarella
Acked-by: Pankaj Gupta
Message-id: 20190208134950.187665-2-sgarz...@redhat.com
Signed-off-by: Stefan Hajnoczi
---
hw/block/virtio-blk.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 9a87b3bfac..b2bb129efa 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -61,7 +61,7 @@ static void virtio_blk_req_complete(VirtIOBlockReq *req,
unsigned char status)
}
static int virtio_blk_handle_rw_error(VirtIOBlockReq *req, int error,
-bool is_read)
+bool is_read, bool acct_failed)
{
BlockErrorAction action = blk_get_error_action(req->dev->blk,
is_read, error);
@@ -75,7 +75,9 @@ static int virtio_blk_handle_rw_error(VirtIOBlockReq *req,
int error,
s->rq = req;
} else if (action == BLOCK_ERROR_ACTION_REPORT) {
virtio_blk_req_complete(req, VIRTIO_BLK_S_IOERR);
-block_acct_failed(blk_get_stats(s->blk), >acct);
+if (acct_failed) {
+block_acct_failed(blk_get_stats(s->blk), >acct);
+}
virtio_blk_free_request(req);
}
@@ -113,7 +115,7 @@ static void virtio_blk_rw_complete(void *opaque, int ret)
* the memory until the request is completed (which will
* happen on the other side of the migration).
*/
-if (virtio_blk_handle_rw_error(req, -ret, is_read)) {
+if (virtio_blk_handle_rw_error(req, -ret, is_read, true)) {
continue;
}
}
@@ -132,7 +134,7 @@ static void virtio_blk_flush_complete(void *opaque, int ret)
aio_context_acquire(blk_get_aio_context(s->conf.conf.blk));
if (ret) {
-if (virtio_blk_handle_rw_error(req, -ret, 0)) {
+if (virtio_blk_handle_rw_error(req, -ret, 0, true)) {
goto out;
}
}
--
2.20.1
[Qemu-devel] [RFC PULL 3/6] virtio-blk: add "discard" and "write-zeroes" properties
From: Stefano Garzarella
In order to avoid migration issues, we enable DISCARD and
WRITE_ZEROES features only for machine type >= 4.0
As discussed with Michael S. Tsirkin and Stefan Hajnoczi on the
list [1], DISCARD operation should not have security implications
(eg. page cache attacks), so we can enable it by default.
[1] https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg00504.html
Suggested-by: Dr. David Alan Gilbert
Reviewed-by: Michael S. Tsirkin
Reviewed-by: Stefan Hajnoczi
Signed-off-by: Stefano Garzarella
Acked-by: Pankaj Gupta
Message-id: 20190208134950.187665-4-sgarz...@redhat.com
Signed-off-by: Stefan Hajnoczi
---
hw/block/virtio-blk.c | 4
hw/core/machine.c | 2 ++
2 files changed, 6 insertions(+)
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 938273c63c..6f2e86264d 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -1026,6 +1026,10 @@ static Property virtio_blk_properties[] = {
DEFINE_PROP_UINT16("queue-size", VirtIOBlock, conf.queue_size, 128),
DEFINE_PROP_LINK("iothread", VirtIOBlock, conf.iothread, TYPE_IOTHREAD,
IOThread *),
+DEFINE_PROP_BIT64("discard", VirtIOBlock, host_features,
+ VIRTIO_BLK_F_DISCARD, true),
+DEFINE_PROP_BIT64("write-zeroes", VirtIOBlock, host_features,
+ VIRTIO_BLK_F_WRITE_ZEROES, true),
DEFINE_PROP_END_OF_LIST(),
};
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 077fbd182a..766ca5899d 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -33,6 +33,8 @@ GlobalProperty hw_compat_3_1[] = {
{ "usb-kbd", "serial", "42" },
{ "usb-mouse", "serial", "42" },
{ "usb-kbd", "serial", "42" },
+{ "virtio-blk-device", "discard", "false" },
+{ "virtio-blk-device", "write-zeroes", "false" },
};
const size_t hw_compat_3_1_len = G_N_ELEMENTS(hw_compat_3_1);
--
2.20.1
Re: [Qemu-devel] [PATCH v3 1/2] pcie: Add a simple PCIe ACS (Access Control Services) helper function
On Tue, 12 Feb 2019 09:07:43 +0100
Knut Omang wrote:
> On Mon, 2019-02-11 at 16:09 -0700, Alex Williamson wrote:
> > On Sun, 10 Feb 2019 07:52:59 +0100
> > Knut Omang wrote:
> >
> > > Add a helper function to add PCIe capability for Access Control Services
> > > (ACS)
> > > ACS support in the associated root port is a prerequisite to be able to do
> > > passthrough of individual functions of a device with VFIO
> > > without Alex Williamson's pcie_acs_override kernel patch or similar
> > > in the guest.
> >
> > This is still incorrect, the ACS override patch is only required for
> > separating multifunction endpoints or multifunction root ports. Single
> > function endpoints are assignable without ACS simply by placing them
> > downstream of a single function root port or directly on the root
> > complex.
>
> Hmm - that was the intended meaning of the comment, but I'll see if I can make
> it more clear by saying it explicitly.
"ACS support... is a prerequisite". Prerequisite: a thing that is
required as a prior condition for something else to happen or exist.
Assignment of individual functions exists today, as is, by using QEMU
to define a PCIe topology that allows the desired grouping. The code
here enables specific topologies, it is clearly not a prerequisite.
> > > Endpoints may also implement an ACS capability, but with
> > > limited features.
> > >
> > > Signed-off-by: Knut Omang
> > > ---
> > > hw/pci/pcie.c | 29 +
> > > include/hw/pci/pcie.h | 6 ++
> > > include/hw/pci/pcie_regs.h | 4
> > > 3 files changed, 39 insertions(+)
> > >
> > > diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> > > index 230478f..509632f 100644
> > > --- a/hw/pci/pcie.c
> > > +++ b/hw/pci/pcie.c
> > > @@ -742,6 +742,14 @@ bool pcie_cap_is_arifwd_enabled(const PCIDevice *dev)
> > > PCI_EXP_DEVCTL2_ARI;
> > > }
> > >
> > > +/* Access Control Services (ACS) */
> > > +void pcie_cap_acs_reset(PCIDevice *dev)
> > > +{
> > > +if (dev->exp.acs_cap) {
> > > +pci_set_word(dev->config + dev->exp.acs_cap + PCI_ACS_CTRL, 0);
> > > +}
> > > +}
> > > +
> > >
> > > /**
> > > * pci express extended capability list management functions
> > > * uint16_t ext_cap_id (16 bit)
> > > @@ -906,3 +914,24 @@ void pcie_ats_init(PCIDevice *dev, uint16_t offset)
> > >
> > > pci_set_word(dev->wmask + dev->exp.ats_cap + PCI_ATS_CTRL, 0x800f);
> > > }
> > > +
> > > +/* ACS (Access Control Services) */
> > > +void pcie_acs_init(PCIDevice *dev, uint16_t offset)
> > > +{
> > > +bool is_pcie_slot = !!object_dynamic_cast(OBJECT(dev),
> > > TYPE_PCIE_SLOT);
> > > +uint16_t cap_bits = 0;
> > > +
> > > +pcie_add_capability(dev, PCI_EXT_CAP_ID_ACS, PCI_ACS_VER, offset,
> > > +PCI_ACS_SIZEOF);
> > > +dev->exp.acs_cap = offset;
> > > +
> > > +if (is_pcie_slot) {
> > > +/* Endpoints may also implement ACS, but these capabilities are
> > > */
> > > +/* only valid for slots: */
> >
> > Not quite, SV, TB, and UF must not be implemented by endpoints, but RR
> > and CR must be implemented by multifunction endpoints that support p2p
> > if they provide an ACS capability.
>
> Hmm - are you ok with setting 0 here as I have done, just amending your
> description to the comment? Then any future emulation that do support p2p
> would have to set the needed bits after calling the init function.
The comment definitely needs work, but I don't know what to do about
single function, non-SR-IOV capable devices calling into this. Per the
spec, these devices cannot implement an ACS capability at all, but will
anyone care if they do? Maybe endpoints need a different init
function. Maybe the capability ID needs to be obscured until
additional functions or an SR-IOV capability are added. I'm not really
concerned that this helper can only indicate lack of p2p between
functions for endpoints, other than the comments being wrong.
> After your previous comments on this, I had a look at Mellanox CX4 and CX5
> which
> are the only devices I could find in the lab that are endpoints and implement
> an
> ACS capability, and neither seems to implement any extra capabilities:
>
> Capabilities: [230 v1] Access Control Services
> ACSCap: SrcValid- TransBlk- ReqRedir- CmpltRedir-
> UpstreamFwd- EgressCtrl- DirectTrans-
> ACSCtl: SrcValid- TransBlk- ReqRedir- CmpltRedir-
> UpstreamFwd- EgressCtrl- DirectTrans-
>
> that was the reason for my choice of value here - after skimming through the
> spec (with my still very limited understanding of the details)
If the above device is a multifunction device or SR-IOV capable device,
this is the correct way to indicate there is no p2p between functions.
This takes advantage of the wording in the spec that certain
capabilities must be implemented by functions
Re: [Qemu-devel] [PATCH 22/25] hw/arm: Express dependencies of nrf51 Kconfig
On 2019-02-11 18:37, Philippe Mathieu-Daudé wrote: > Hi Thomas, > > On 2/9/19 7:39 AM, Thomas Huth wrote: >> Add Kconfig dependencies for the NRF51 / microbit machine. >> >> Signed-off-by: Thomas Huth >> --- >> hw/arm/Kconfig | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig >> index c4b3cd2..aad2dd3 100644 >> --- a/hw/arm/Kconfig >> +++ b/hw/arm/Kconfig >> @@ -379,6 +379,8 @@ config FSL_IMX6UL >> >> config NRF51_SOC >> bool >> +select I2C >> +select ARM_V7M > > Don't we need a microbit config too? As long as we do not have another board that uses the NRF51-SoC, it does not matter that much ... but yes, it's likely cleaner to have a proper switch for the machine right from the start. I'll fix it in v2 ... Thomas
[Qemu-devel] [PATCH 0/7] slirp: make it a standalone project
Hi,
As discussed earlier in "[PATCH for-3.2 00/41] RFC: slirp: make it
again a standalone project" and other threads, it would be useful to
make slirp a separate project.
In the previous thread "[PATCH slirp 0/5] Make it a standalone
project", it was recommended by Peter that we keep the submodule build
for a while, until libslirp is released with stability commitments.
This patch series is to be applied on QEMU source tree, and modify
slirp/ to make it easily submodule-able or build QEMU against a system
installed version.
Based-on: <20190212160953.29051-1-marcandre.lur...@redhat.com>
Marc-André Lureau (7):
slirp: adapt a subset of QEMU vmstate code
slirp: use libslirp migration code
slirp: use "slirp_" prefix for inet_aton() win32 implementation
slirp: move sources to src/ subdirectory
slirp: add a standalone Makefile
build-sys: link with slirp as an external project
slirp: remove QEMU Makefile.objs
include/migration/qemu-file-types.h | 2 +
migration/qemu-file.h | 1 -
slirp/{ => src}/bootp.h | 0
slirp/{ => src}/debug.h | 0
slirp/{ => src}/dhcpv6.h| 0
slirp/{ => src}/if.h| 0
slirp/{ => src}/ip.h| 0
slirp/{ => src}/ip6.h | 0
slirp/{ => src}/ip6_icmp.h | 0
slirp/{ => src}/ip_icmp.h | 0
slirp/{ => src}/libslirp.h | 9 +
slirp/{ => src}/main.h | 0
slirp/{ => src}/mbuf.h | 0
slirp/{ => src}/misc.h | 0
slirp/{ => src}/ncsi-pkt.h | 0
slirp/{ => src}/qtailq.h| 0
slirp/{ => src}/sbuf.h | 0
slirp/{ => src}/slirp.h | 0
slirp/{ => src}/socket.h| 0
slirp/src/state.h | 0
slirp/src/stream.h | 34 +++
slirp/{ => src}/tcp.h | 0
slirp/{ => src}/tcp_timer.h | 0
slirp/{ => src}/tcp_var.h | 0
slirp/{ => src}/tcpip.h | 0
slirp/{ => src}/tftp.h | 0
slirp/{ => src}/udp.h | 0
slirp/{ => src}/util.h | 4 +-
slirp/src/vmstate.h | 396 +++
slirp/state.h | 9 -
net/slirp.c | 57 +++-
slirp/{ => src}/arp_table.c | 0
slirp/{ => src}/bootp.c | 0
slirp/{ => src}/cksum.c | 0
slirp/{ => src}/dhcpv6.c| 0
slirp/{ => src}/dnssearch.c | 0
slirp/{ => src}/if.c| 0
slirp/{ => src}/ip6_icmp.c | 0
slirp/{ => src}/ip6_input.c | 0
slirp/{ => src}/ip6_output.c| 0
slirp/{ => src}/ip_icmp.c | 0
slirp/{ => src}/ip_input.c | 0
slirp/{ => src}/ip_output.c | 0
slirp/{ => src}/mbuf.c | 0
slirp/{ => src}/misc.c | 0
slirp/{ => src}/ncsi.c | 0
slirp/{ => src}/ndp_table.c | 0
slirp/{ => src}/sbuf.c | 0
slirp/{ => src}/slirp.c | 9 -
slirp/{ => src}/socket.c| 0
slirp/{ => src}/state.c | 52 ++--
slirp/src/stream.c | 119 +
slirp/{ => src}/tcp_input.c | 0
slirp/{ => src}/tcp_output.c| 0
slirp/{ => src}/tcp_subr.c | 0
slirp/{ => src}/tcp_timer.c | 0
slirp/{ => src}/tftp.c | 0
slirp/{ => src}/udp.c | 0
slirp/{ => src}/udp6.c | 0
slirp/{ => src}/util.c | 4 +-
slirp/src/vmstate.c | 401
util/main-loop.c| 2 -
vl.c| 3 -
Makefile| 8 +-
Makefile.objs | 1 -
Makefile.target | 5 +-
configure | 65 -
net/Makefile.objs | 2 +
slirp/Makefile | 47
slirp/Makefile.objs | 34 ---
util/Makefile.objs | 1 +
71 files changed, 1162 insertions(+), 103 deletions(-)
rename slirp/{ => src}/bootp.h (100%)
rename slirp/{ => src}/debug.h (100%)
rename slirp/{ => src}/dhcpv6.h (100%)
rename slirp/{ => src}/if.h (100%)
rename slirp/{ => src}/ip.h (100%)
rename slirp/{ => src}/ip6.h (100%)
rename slirp/{ => src}/ip6_icmp.h (100%)
rename slirp/{ => src}/ip_icmp.h (100%)
rename slirp/{ => src}/libslirp.h (93%)
rename slirp/{ => src}/main.h (100%)
rename slirp/{ => src}/mbuf.h (100%)
rename slirp/{ => src}/misc.h (100%)
rename slirp/{ => src}/ncsi-pkt.h (100%)
rename slirp/{ => src}/qtailq.h (100%)
rename slirp/{ => src}/sbuf.h (100%)
rename slirp/{ => src}/slirp.h (100%)
rename slirp/{ => src}/socket.h (100%)
create mode 100644 slirp/src/state.h
create mode 100644 slirp/src/stream.h
rename slirp/{ => src}/tcp.h (100%)
rename slirp/{ => src}/tcp_timer.h (100%)
Re: [Qemu-devel] [Qemu-ppc] [PATCH] cuda: decrease time delay before raising VIA SR interrupt
On 12/02/2019 11:03, BALATON Zoltan wrote:
> On Tue, 12 Feb 2019, Mark Cave-Ayland wrote:
>> On 11/02/2019 23:35, Philippe Mathieu-Daudé wrote:
>>
>>> Hi Mark,
>>>
>>> On 2/10/19 6:44 PM, Mark Cave-Ayland wrote:
In order to handle a race condition in MacOS 9, a delay was introduced when
raising the VIA SR interrupt inspired by similar code in MacOnLinux.
During original testing of the MacOS 9 patches it was found that the 30us
delay used in MacOnLinux did not work reliably within QEMU, and a value of
300us was required to function correctly.
Recent experiments have shown that the previous reliability issues are no
longer present, and this value can be reduced down to 20us with no apparent
ill effects in my local tests. This has the benefit of considerably
improving
the responsiveness of the ADB keyboard and mouse with the guest.
Signed-off-by: Mark Cave-Ayland
---
hw/misc/macio/cuda.c | 11 +--
1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c
index c4f7a2f39b..3febacdd1e 100644
--- a/hw/misc/macio/cuda.c
+++ b/hw/misc/macio/cuda.c
@@ -97,17 +97,8 @@ static void cuda_set_sr_int(void *opaque)
static void cuda_delay_set_sr_int(CUDAState *s)
{
- MOS6522CUDAState *mcs = >mos6522_cuda;
- MOS6522State *ms = MOS6522(mcs);
- MOS6522DeviceClass *mdc = MOS6522_DEVICE_GET_CLASS(ms);
int64_t expire;
- if (ms->dirb == 0xff || s->sr_delay_ns == 0) {
- /* Disabled or not in Mac OS, fire the IRQ directly */
- mdc->set_sr_int(ms);
- return;
- }
>>>
>>> The change of sr_delay_ns below is well explained, but I don't
>>> understand why you remove the previous if().
>>
>> IIRC it was a hack by Alex to try and restrict the delay on the interrupt
>> just to
>> MacOS instead of Linux, but with the reduced value it doesn't really matter
>> any more.
>
> If this delay is to prevent a bug which only happens in MacOS then that's the
> hack
> not the normal code path to run without the delay that you've just removed.
> So maybe
> this should be kept if possible to avoid unecessary delays for other guests.
> (Although if this only affects mac99,via=cuda but not mac99,via=pmu then I
> don't care
> much as long as pmu works.)
Well the reality is that the detection above doesn't actually seem to work
anyway -
at least a quick boot test with Linux, MacOS X and MacOS 9 with a printf()
added into
the if() shows nothing firing once the kernel takes over. So the slow path with
the
delay included was always being taken within the OS anyway.
And indeed, the code doesn't affect pmu so you won't see any difference there.
>> As a plus it also prevents a guest OS from accidentally triggering the hack
>> whilst
>> programming the VIA port.
>
> That may be a problem though. What's the issue exactly? Why is the delay
> needed in
> the first place?
It's some kind of racy polling with OS 9 (I wasn't involved in the technical
details,
sorry) which causes OS 9 to hang on boot if the delay isn't present. And even
better
the slow path that was previously always being taken has now been reduced from
300us
to 30us so whichever way you look at it, having this patch applied is a win.
ATB,
Mark.
Re: [Qemu-devel] [qemu-s390x] [PATCH 06/15] s390-bios: Clean up cio.h
On 2019-02-04 11:48, Cornelia Huck wrote:
> On Tue, 29 Jan 2019 08:29:13 -0500
> "Jason J. Herne" wrote:
>
>> Add proper typedefs to all structs and modify all bit fields to use
>> consistent
>> formatting.
>>
>> Signed-off-by: Jason J. Herne
>> Reviewed-by: Collin Walling
>> ---
>> pc-bios/s390-ccw/cio.h | 86
>> ++---
>> pc-bios/s390-ccw/s390-ccw.h | 8 -
>> 2 files changed, 43 insertions(+), 51 deletions(-)
>>
>> diff --git a/pc-bios/s390-ccw/cio.h b/pc-bios/s390-ccw/cio.h
>> index 1a0795f..a48eee5 100644
>> --- a/pc-bios/s390-ccw/cio.h
>> +++ b/pc-bios/s390-ccw/cio.h
>> @@ -53,12 +53,12 @@ struct schib_config {
>> __u64 mba;
>> __u32 intparm;
>> __u16 mbi;
>> -__u32 isc:3;
>> -__u32 ena:1;
>> -__u32 mme:2;
>> -__u32 mp:1;
>> -__u32 csense:1;
>> -__u32 mbfc:1;
>> +__u32 isc: 3;
>> +__u32 ena: 1;
>> +__u32 mme: 2;
>> +__u32 mp : 1;
>> +__u32 csense : 1;
>> +__u32 mbfc : 1;
>> } __attribute__ ((packed));
>
> This seems to make checkpatch unhappy... maybe consolidate to the other
> formatting instead?
Yeah, you get lots of these errors this way:
ERROR: spaces prohibited around that ':' (ctx:WxW)
#141: FILE: pc-bios/s390-ccw/cio.h:56:
+__u32 isc: 3;
^
ERROR: spaces prohibited around that ':' (ctx:WxW)
#142: FILE: pc-bios/s390-ccw/cio.h:57:
+__u32 ena: 1;
^
ERROR: spaces prohibited around that ':' (ctx:WxW)
#143: FILE: pc-bios/s390-ccw/cio.h:58:
+__u32 mme: 2;
^
...
Not sure whether it's a checkpatch warning or really our official coding
style, but anyway, I'd rather prefer to keep checkpatch calm here and
thus no spaces around the ':' please.
Thomas
Re: [Qemu-devel] [PULL 1/7] audio: fix pc speaker init
On 12.02.19 13:20, David Hildenbrand wrote:
> On 12.02.19 13:08, Philippe Mathieu-Daudé wrote:
>> Hi David,
>>
>> On 2/12/19 12:47 PM, David Hildenbrand wrote:
>>> On 24.01.19 14:20, Gerd Hoffmann wrote:
Get rid of the pcspk_state global, allow pc speaker
be added using "-device isa-pcspk".
Signed-off-by: Gerd Hoffmann
Reviewed-by: Philippe Mathieu-Daudé
Message-id: 20190124110810.1040-1-kra...@redhat.com
---
hw/audio/pcspk.c | 35 +++
1 file changed, 15 insertions(+), 20 deletions(-)
diff --git a/hw/audio/pcspk.c b/hw/audio/pcspk.c
index 908696d483..b80a62ce90 100644
--- a/hw/audio/pcspk.c
+++ b/hw/audio/pcspk.c
@@ -57,7 +57,6 @@ typedef struct {
} PCSpkState;
static const char *s_spk = "pcspk";
-static PCSpkState *pcspk_state;
static inline void generate_samples(PCSpkState *s)
{
@@ -111,22 +110,6 @@ static void pcspk_callback(void *opaque, int free)
}
}
-static int pcspk_audio_init(ISABus *bus)
-{
-PCSpkState *s = pcspk_state;
-struct audsettings as = {PCSPK_SAMPLE_RATE, 1, AUD_FMT_U8, 0};
-
-AUD_register_card(s_spk, >card);
-
-s->voice = AUD_open_out(>card, s->voice, s_spk, s, pcspk_callback,
);
-if (!s->voice) {
-AUD_log(s_spk, "Could not open voice\n");
-return -1;
-}
-
-return 0;
-}
-
static uint64_t pcspk_io_read(void *opaque, hwaddr addr,
unsigned size)
{
@@ -179,12 +162,20 @@ static void pcspk_initfn(Object *obj)
static void pcspk_realizefn(DeviceState *dev, Error **errp)
{
+struct audsettings as = {PCSPK_SAMPLE_RATE, 1, AUD_FMT_U8, 0};
ISADevice *isadev = ISA_DEVICE(dev);
PCSpkState *s = PC_SPEAKER(dev);
isa_register_ioport(isadev, >ioport, s->iobase);
-pcspk_state = s;
+AUD_register_card(s_spk, >card);
+
+s->voice = AUD_open_out(>card, s->voice, s_spk, s, pcspk_callback,
);
+if (!s->voice) {
+error_setg(errp, "Initializing audio voice failed");
+AUD_remove_card(>card);
+return;
+}
}
static bool migrate_needed(void *opaque)
@@ -221,8 +212,6 @@ static void pcspk_class_initfn(ObjectClass *klass,
void *data)
set_bit(DEVICE_CATEGORY_SOUND, dc->categories);
dc->vmsd = _spk;
dc->props = pcspk_properties;
-/* Reason: realize sets global pcspk_state */
-dc->user_creatable = false;
}
static const TypeInfo pcspk_info = {
@@ -233,6 +222,12 @@ static const TypeInfo pcspk_info = {
.class_init = pcspk_class_initfn,
};
+static int pcspk_audio_init(ISABus *bus)
+{
+isa_create_simple(bus, TYPE_PC_SPEAKER);
+return 0;
+}
+
static void pcspk_register(void)
{
type_register_static(_info);
>>>
>>> I suddenly get (under fedora 28)
>>>
>>> ALSA lib pulse.c:243:(pulse_connect) PulseAudio: Unable to connect:
>>> Connection refused
>>>
>>> alsa: Could not initialize DAC
>>> alsa: Failed to open `default':
>>> alsa: Reason: Connection refused
>>> ALSA lib pulse.c:243:(pulse_connect) PulseAudio: Unable to connect:
>>> Connection refused
>>>
>>> alsa: Could not initialize DAC
>>> alsa: Failed to open `default':
>>> alsa: Reason: Connection refused
>>
>> This ALSA problem seems on your side.
>>
>>> audio: Failed to create voice `pcspk'
>>> qemu-system-x86_64: Initialization of device isa-pcspk failed:
>>> Initializing audio voice failed
>>
>> Previously the errors would be ignored and QEMU would start.
>
> I just did a fedora 28 uupdate + reboot. Problem still exists. Would be
> strange if only I would be hitting this problem with stock alsa libraries.
>
>>
>>>
>>>
>>> With
>>>
>>> sudo x86_64-softmmu/qemu-system-x86_64 \
>>> --enable-kvm \
>>> -m 4G,maxmem=40G,slots=2 \
>>> -smp sockets=2,cores=2 \
>>> -numa node,nodeid=0,cpus=0-1 -numa node,nodeid=1,cpus=2-3 \
>>> -kernel /boot/vmlinuz-4.19.6-200.fc28.x86_64 \
>>> -append "console=ttyS0 rd.shell rd.luks=0 rd.lvm=0 rd.md=0 rd.dm=0" \
>>> -initrd /boot/initramfs-4.19.6-200.fc28.x86_64.img \
>>> -machine pc,nvdimm \
>>> -nographic \
>>> -nodefaults \
>>> -chardev stdio,id=serial \
>>> --trace events=to_trace \
>>> -device isa-serial,chardev=serial \
>>> -chardev socket,id=monitor,path=/var/tmp/monitor,server,nowait \
>>> -mon chardev=monitor,mode=readline
>>>
>>>
>>> Could this be related to this patch? (or is maybe something about my
>>> system messed up? will try rebooting, but other audio - e.g. via firefox
>>> - works fine)
>>
>> Does your Firefox uses ALSA? The default install uses PulseAudio.
>
>
Re: [Qemu-devel] [PULL 0/3] Block patches
On Tue, 12 Feb 2019 at 04:01, Stefan Hajnoczi wrote: > > The following changes since commit 22c5f446514a2a4bb0dbe1fea26713da92fc85fa: > > Merge remote-tracking branch 'remotes/rth/tags/pull-tcg-20190211' into > staging (2019-02-11 17:04:57 +) > > are available in the Git repository at: > > git://github.com/stefanha/qemu.git tags/block-pull-request > > for you to fetch changes up to 9a6719d572e99a4e79f589d0b73f7475b86f982d: > > virtio-blk: cleanup using VirtIOBlock *s and VirtIODevice *vdev (2019-02-12 > 11:49:17 +0800) > > > Pull request > > Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/4.0 for any user-visible changes. -- PMM
[Qemu-devel] [Bug 1808928] Re: Bitmap Extra data is not supported
as far as i know nothing happened. it had worked normally while instance was running. For a reason, instance is shutdown, then it never open again. i have some backups, i tried to return previous backups. But they also gave same error. Thanks to replication i could get it back. i copied image from replication site... as i said "qemu-img" command completely unusable on that image. There should be another mechanism. It blocks everything. I try to edit header to remove extra data. But i could not find header information on website. thanks -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1808928 Title: Bitmap Extra data is not supported Status in QEMU: New Bug description: i am using dirty bitmaps and drive-backup. It works as aspected. Lately, i encounter a disastrous error. There is not any information about that situation. I cannot reach/open/attach/info or anything with a qcow2 file. virsh version Compiled against library: libvirt 4.10.0 Using library: libvirt 4.10.0 Using API: QEMU 4.10.0 Running hypervisor: QEMU 2.12.0 "qemu-img: Could not open '/var/lib/libvirt/images/test.qcow2': Bitmap extra data is not supported" what is that mean? what should i do? i cannot remove bitmap. i cannot open image or query. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1808928/+subscriptions
Re: [Qemu-devel] [PULL 00/25] pci, pc, virtio: fixes, cleanups, features
On Tue, Feb 12, 2019 at 02:15:36PM +0100, Philippe Mathieu-Daudé wrote: > On 2/12/19 2:04 PM, Michael S. Tsirkin wrote: > > On Tue, Feb 12, 2019 at 11:39:21AM +0100, Philippe Mathieu-Daudé wrote: > >> On 2/12/19 8:11 AM, Peter Xu wrote: > >>> On Tue, Feb 05, 2019 at 11:06:42AM -0500, Michael S. Tsirkin wrote: > > OK I reverted the whole part dealing with vhost-user and reposted. > >>> > >>> I noticed that the merged pull request could possibly have squashed > >>> the below two patches (in previous pull) into one super patch > >>> (a56de056c91f87e1e): > >>> > >>> i386/kvm: ignore masked irqs when update msi routes > >>> contrib/vhost-user-blk: fix the compilation issue > >>> > >>> Here, the first patch lost its commit message, and the last patch lost > >>> its real author. ;) > >> > >> I suggest we revert a56de056c9 ASAP and reapply the both patches, this > >> will ease cherry-picking/downstream workflow. > > > > I don't see why does it help upstream. > > I'd have suggested the same if I had no idea what 'downstream workflow' > mean, simply to keep the tree clear and avoid to have unrelated changes > squashed altogether. > Commit a56de056c9 really looks messy. MSI/MSIX changes described by "fix > vhost-user-blk compilation". > Hopefully it won't trigger any problem which requires bisecting to it, > then contact Changpeng Liu asking him what he intented to do with his > commit. > Your call anyway :) > > Regards, > > Phil. OK these are good points. I'm not sure what happened but it looks like I screwed up when resolving some conflicts. Care posting a patchset looking sane? -- MST
Re: [Qemu-devel] [PATCH v2 4/4] migration: Add capabilities validation
11.02.2019, 16:30, "Dr. David Alan Gilbert" :
> * Yury Kotov (yury-ko...@yandex-team.ru) wrote:
>> Currently we don't check which capabilities set in the source QEMU.
>> We just expect that the target QEMU has the same enabled capabilities.
>>
>> Add explicit validation for capabilities to make sure that the target VM
>> has them too. This is enabled for only new capabilities to keep compatibily.
>
> I'd rather keep the capaiblities on the wire as strings, rather than
> indexes; indexes are too delicate.
>
It seems that strings also have a problem. E.g. when we remove 'x-' prefix from
x-some-capability. But I don't insist.
> I guess we also have to think about what happens when new capabilities
> are added; what happens when we migrate to an older qemu that doesn't
> know about that capability?
> What happens when we migrate to a newer capability which thinks you
> forgot to send that capability across?
>
I thought about such cases:
> what happens when new capabilities are added?
If new capability should be validated, then source will send it, target will
expect it. Otherwise, nothing will happen.
> what happens when we migrate to an older qemu that doesn't
> know about that capability?
Incoming migration will be failed as expected. If old qemu doesn't know the
capability, therefore qemu doesn't support it. In this case, user should disable
the capability on source before migration.
> What happens when we migrate to a newer capability which thinks you
> forgot to send that capability across?
Sorry, I'm not sure that I understood correctly. It seems that it's the same
case as the previous one.
> While we're on capabilities, I think you also need to check if the
> skip-shared is enabled with postcopy; I don't think they'll work
> together.
>
I agree, postcopy and skip-shared shouldn't be enabled together.
Will add a check in v3.
> Dave
>
>> Signed-off-by: Yury Kotov
>> ---
>> migration/savevm.c | 101 +
>> 1 file changed, 101 insertions(+)
>>
>> diff --git a/migration/savevm.c b/migration/savevm.c
>> index 322660438d..9603a38bca 100644
>> --- a/migration/savevm.c
>> +++ b/migration/savevm.c
>> @@ -57,6 +57,7 @@
>> #include "sysemu/replay.h"
>> #include "qjson.h"
>> #include "migration/colo.h"
>> +#include "qemu/bitmap.h"
>>
>> #ifndef ETH_P_RARP
>> #define ETH_P_RARP 0x8035
>> @@ -316,6 +317,8 @@ typedef struct SaveState {
>> uint32_t len;
>> const char *name;
>> uint32_t target_page_bits;
>> + uint32_t caps_count;
>> + uint8_t *capabilities;
>> } SaveState;
>>
>> static SaveState savevm_state = {
>> @@ -323,15 +326,51 @@ static SaveState savevm_state = {
>> .global_section_id = 0,
>> };
>>
>> +static bool should_validate_capability(int capability)
>> +{
>> + assert(capability >= 0 && capability < MIGRATION_CAPABILITY__MAX);
>> + /* Validate only new capabilities to keep compatibility. */
>> + switch (capability) {
>> + case MIGRATION_CAPABILITY_X_IGNORE_SHARED:
>> + return true;
>> + default:
>> + return false;
>> + }
>> +}
>> +
>> +static uint32_t get_validatable_capabilities_count(void)
>> +{
>> + MigrationState *s = migrate_get_current();
>> + uint32_t result = 0;
>> + int i;
>> + for (i = 0; i < MIGRATION_CAPABILITY__MAX; i++) {
>> + if (should_validate_capability(i) && s->enabled_capabilities[i]) {
>> + result++;
>> + }
>> + }
>> + return result;
>> +}
>> +
>> static int configuration_pre_save(void *opaque)
>> {
>> SaveState *state = opaque;
>> const char *current_name = MACHINE_GET_CLASS(current_machine)->name;
>> + MigrationState *s = migrate_get_current();
>> + int i, j;
>>
>> state->len = strlen(current_name);
>> state->name = current_name;
>> state->target_page_bits = qemu_target_page_bits();
>>
>> + state->caps_count = get_validatable_capabilities_count();
>> + state->capabilities = g_renew(uint8_t, state->capabilities,
>> + state->caps_count);
>> + for (i = j = 0; i < MIGRATION_CAPABILITY__MAX; i++) {
>> + if (should_validate_capability(i) && s->enabled_capabilities[i]) {
>> + state->capabilities[j++] = i;
>> + }
>> + }
>> +
>> return 0;
>> }
>>
>> @@ -347,6 +386,45 @@ static int configuration_pre_load(void *opaque)
>> return 0;
>> }
>>
>> +static bool configuration_validate_capabilities(SaveState *state)
>> +{
>> + bool ret = true;
>> + MigrationState *s = migrate_get_current();
>> + unsigned long *source_caps_bm;
>> + int i;
>> +
>> + source_caps_bm = bitmap_new(MIGRATION_CAPABILITY__MAX);
>> + for (i = 0; i < state->caps_count; i++) {
>> + int capability = state->capabilities[i];
>> + if (capability >= MIGRATION_CAPABILITY__MAX) {
>> + error_report("Received unknown capability %d", capability);
>> + ret = false;
>> + } else {
>> + set_bit(capability, source_caps_bm);
>> + }
>> + }
>> +
>> + for (i = 0; i < MIGRATION_CAPABILITY__MAX; i++) {
>> + bool source_state,
[Qemu-devel] [RFC PULL 3/6] virtio-blk: add "discard" and "write-zeroes" properties
From: Stefano Garzarella
In order to avoid migration issues, we enable DISCARD and
WRITE_ZEROES features only for machine type >= 4.0
As discussed with Michael S. Tsirkin and Stefan Hajnoczi on the
list [1], DISCARD operation should not have security implications
(eg. page cache attacks), so we can enable it by default.
[1] https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg00504.html
Suggested-by: Dr. David Alan Gilbert
Reviewed-by: Michael S. Tsirkin
Reviewed-by: Stefan Hajnoczi
Signed-off-by: Stefano Garzarella
Acked-by: Pankaj Gupta
Message-id: 20190208134950.187665-4-sgarz...@redhat.com
Signed-off-by: Stefan Hajnoczi
---
hw/block/virtio-blk.c | 4
hw/core/machine.c | 2 ++
2 files changed, 6 insertions(+)
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 938273c63c..6f2e86264d 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -1026,6 +1026,10 @@ static Property virtio_blk_properties[] = {
DEFINE_PROP_UINT16("queue-size", VirtIOBlock, conf.queue_size, 128),
DEFINE_PROP_LINK("iothread", VirtIOBlock, conf.iothread, TYPE_IOTHREAD,
IOThread *),
+DEFINE_PROP_BIT64("discard", VirtIOBlock, host_features,
+ VIRTIO_BLK_F_DISCARD, true),
+DEFINE_PROP_BIT64("write-zeroes", VirtIOBlock, host_features,
+ VIRTIO_BLK_F_WRITE_ZEROES, true),
DEFINE_PROP_END_OF_LIST(),
};
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 077fbd182a..766ca5899d 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -33,6 +33,8 @@ GlobalProperty hw_compat_3_1[] = {
{ "usb-kbd", "serial", "42" },
{ "usb-mouse", "serial", "42" },
{ "usb-kbd", "serial", "42" },
+{ "virtio-blk-device", "discard", "false" },
+{ "virtio-blk-device", "write-zeroes", "false" },
};
const size_t hw_compat_3_1_len = G_N_ELEMENTS(hw_compat_3_1);
--
2.20.1
[Qemu-devel] [RFC PULL 0/6] Block sgarzare test patches
The following changes since commit 22c5f446514a2a4bb0dbe1fea26713da92fc85fa: Merge remote-tracking branch 'remotes/rth/tags/pull-tcg-20190211' into staging (2019-02-11 17:04:57 +) are available in the Git repository at: git://github.com/stefanha/qemu.git tags/block-sgarzare-test-pull-request for you to fetch changes up to bcfdf3d6971e2fd3d424d0c2ea80fdccc35d3dda: tests/virtio-blk: add test for WRITE_ZEROES command (2019-02-12 22:00:09 +0800) Block pull request for testing Peter hit a virtio-blk-test failure caused by the new DISCARD/WRITE_ZEROES patches that Stefano and I have been unable to reproduce. Here are the patches so they can be tested again in Peter's environment. Stefano Garzarella (6): virtio-blk: add acct_failed param to virtio_blk_handle_rw_error() virtio-blk: add host_features field in VirtIOBlock virtio-blk: add "discard" and "write-zeroes" properties virtio-blk: add DISCARD and WRITE_ZEROES features tests/virtio-blk: change assert on data_size in virtio_blk_request() tests/virtio-blk: add test for WRITE_ZEROES command include/hw/virtio/virtio-blk.h | 5 +- hw/block/virtio-blk.c | 214 +++-- hw/core/machine.c | 2 + tests/virtio-blk-test.c| 75 +++- 4 files changed, 282 insertions(+), 14 deletions(-) -- 2.20.1
[Qemu-devel] [RFC PULL 5/6] tests/virtio-blk: change assert on data_size in virtio_blk_request()
From: Stefano Garzarella
The size of data in the virtio_blk_request must be a multiple
of 512 bytes for IN and OUT requests, or a multiple of the size
of struct virtio_blk_discard_write_zeroes for DISCARD and
WRITE_ZEROES requests.
Reviewed-by: Michael S. Tsirkin
Reviewed-by: Stefan Hajnoczi
Reviewed-by: Thomas Huth
Signed-off-by: Stefano Garzarella
Acked-by: Pankaj Gupta
Message-id: 20190208134950.187665-6-sgarz...@redhat.com
Signed-off-by: Stefan Hajnoczi
---
tests/virtio-blk-test.c | 15 ++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
index 04c608764b..0739498da7 100644
--- a/tests/virtio-blk-test.c
+++ b/tests/virtio-blk-test.c
@@ -144,7 +144,20 @@ static uint64_t virtio_blk_request(QGuestAllocator *alloc,
QVirtioDevice *d,
uint64_t addr;
uint8_t status = 0xFF;
-g_assert_cmpuint(data_size % 512, ==, 0);
+switch (req->type) {
+case VIRTIO_BLK_T_IN:
+case VIRTIO_BLK_T_OUT:
+g_assert_cmpuint(data_size % 512, ==, 0);
+break;
+case VIRTIO_BLK_T_DISCARD:
+case VIRTIO_BLK_T_WRITE_ZEROES:
+g_assert_cmpuint(data_size %
+ sizeof(struct virtio_blk_discard_write_zeroes), ==,
0);
+break;
+default:
+g_assert_cmpuint(data_size, ==, 0);
+}
+
addr = guest_alloc(alloc, sizeof(*req) + data_size);
virtio_blk_fix_request(d, req);
--
2.20.1
Re: [Qemu-devel] [PATCH 01/13] block: Allow freezing BdrvChild links
Am 17.01.2019 um 16:33 hat Alberto Garcia geschrieben:
> Our permission system is useful to define what operations are allowed
> on a certain block node and includes things like BLK_PERM_WRITE or
> BLK_PERM_RESIZE among others.
>
> One of the permissions is BLK_PERM_GRAPH_MOD which allows "changing
> the node that this BdrvChild points to". The exact meaning of this has
> never been very clear, but it can be understood as "change any of the
> links connected to the node". This can be used to prevent changing a
> backing link, but it's too coarse.
>
> This patch adds a new 'frozen' attribute to BdrvChild, which forbids
> detaching the link from the node it points to, and new API to freeze
> and unfreeze a backing chain.
>
> Signed-off-by: Alberto Garcia
> ---
> block.c | 66
> +++
> include/block/block.h | 5
> include/block/block_int.h | 5
> 3 files changed, 76 insertions(+)
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index f605622216..fd0e88d17a 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -661,6 +661,11 @@ struct BdrvChild {
> */
> uint64_t shared_perm;
>
> +/*
> + * This link is frozen: the child cannot be detached from the parent.
> + */
> +bool frozen;
> +
> QLIST_ENTRY(BdrvChild) next;
> QLIST_ENTRY(BdrvChild) next_parent;
> };
Is this enough, or should it prevent both detaching from the parent _and_
changing the child node it points to?
> diff --git a/block.c b/block.c
> index 4f5ff2cc12..51fac086c7 100644
> --- a/block.c
> +++ b/block.c
> @@ -2062,6 +2062,8 @@ static void bdrv_replace_child_noperm(BdrvChild *child,
> BlockDriverState *old_bs = child->bs;
> int i;
>
> +assert(!child->frozen);
> +
> if (old_bs && new_bs) {
> assert(bdrv_get_aio_context(old_bs) == bdrv_get_aio_context(new_bs));
> }
Okay, this does forbid both, so I think it's just the comment that needs
to be changed in the hunk above.
In order to avoid running into an assertion failure, we need to detect
the situation in all callers, or audit that it can never happen:
* bdrv_replace_child()
* bdrv_root_attach_child(): Creates a new BdrvChild, never frozen
* bdrv_detach_child()
* bdrv_root_unref_child()
* blk_remove_bs(): Not a backing child, safe
* block_job_remove_all_bdrv(): Not a backing child, safe
* bdrv_unref_child()
* bdrv_set_backing_hd(): Adds a check
* bdrv_close(): Not a backing child, safe
* bdrv_open_driver(): Not a backing child, safe
* bdrv_drop_intermediate(): Adds a check, though see below
* bdrv_replace_node(): Missing check?
* bdrv_append()
For example, mirror_start_job() calls it to insert the
mirror_top_bs node above the source, which could be anywhere in
the graph. This may involve changing the target of backing links.
* Other callers looks questionable, too
bdrv_replace_node() can return an error, so I think we should add a
check there.
For the bdrv_replace_child() call chain, the state looks better, but it
was tedious to verify and future additions could easily forget to add
the check. I wonder if we should allow some function there to return
errors so that we don't have to add the checks in the outermost
functions of the call chain.
> @@ -3813,6 +3819,62 @@ BlockDriverState *bdrv_find_base(BlockDriverState *bs)
> }
>
> /*
> + * Return true if at least one of the backing links between @bs and
> + * @base is frozen. @errp is set if that's the case.
> + */
> +bool bdrv_is_backing_chain_frozen(BlockDriverState *bs, BlockDriverState
> *base,
> + Error **errp)
> +{
> +BlockDriverState *i;
> +
> +for (i = bs; i != base && i->backing; i = backing_bs(i)) {
> +if (i->backing->frozen) {
> +error_setg(errp, "Cannot remove link from '%s' to '%s'",
Maybe s/remove/change/?
Might also be worth including the BdrvChild name in the error message.
> + i->node_name, backing_bs(i)->node_name);
> +return true;
> +}
> +}
> +
> +return false;
> +}
> @@ -3861,6 +3923,10 @@ int bdrv_drop_intermediate(BlockDriverState *top,
> BlockDriverState *base,
> goto exit;
> }
>
> +if (bdrv_is_backing_chain_frozen(top, base, NULL)) {
> +goto exit;
> +}
> +
> /* If 'base' recursively inherits from 'top' then we should set
> * base->inherits_from to top->inherits_from after 'top' and all
> * other intermediate nodes have been dropped.
bdrv_drop_intermediate() doesn't change the links between in the chain
between top and base, but the links between the parents of top and top
are changed to point to base instead.
I think this is checking the wrong thing.
Kevin
Re: [Qemu-devel] [PATCH v3] virtio-blk: set correct config size for the host driver
On Tue, Feb 12, 2019 at 11:19:49PM +0800, Changpeng Liu wrote:
> Commit caa1ee43 "vhost-user-blk: add discard/write zeroes features
> support" added fields to struct virtio_blk_config. This changes
> the size of the config space and breaks migration from QEMU 3.1
> and older:
>
> qemu-system-ppc64: get_pci_config_device: Bad config data: i=0x10 read: 41
> device: 1 cmask: ff wmask: 80 w1cmask:0
> qemu-system-ppc64: Failed to load PCIDevice:config
> qemu-system-ppc64: Failed to load virtio-blk:virtio
> qemu-system-ppc64: error while loading state for instance 0x0 of device
> 'pci@8002000:01.0/virtio-blk'
> qemu-system-ppc64: load of migration failed: Invalid argument
>
> Since virtio-blk doesn't support the "discard" and "write zeroes"
> features, it shouldn't even expose the associated fields in the
> config space actually. Just include all fields up to num_queues to
> match QEMU 3.1 and older.
>
> Signed-off-by: Changpeng Liu
OK almost.
> ---
> hw/block/virtio-blk.c | 14 ++
> 1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
> index 9a87b3b..0ff5315 100644
> --- a/hw/block/virtio-blk.c
> +++ b/hw/block/virtio-blk.c
> @@ -28,6 +28,10 @@
> #include "hw/virtio/virtio-bus.h"
> #include "hw/virtio/virtio-access.h"
>
> +/* We don't support discard yet, hide associated config fields. */
> +#define VIRTIO_BLK_CFG_SIZE offsetof(struct virtio_blk_config, \
> + max_discard_sectors)
> +
> static void virtio_blk_init_request(VirtIOBlock *s, VirtQueue *vq,
> VirtIOBlockReq *req)
> {
> @@ -761,7 +765,9 @@ static void virtio_blk_update_config(VirtIODevice *vdev,
> uint8_t *config)
> blkcfg.alignment_offset = 0;
> blkcfg.wce = blk_enable_write_cache(s->blk);
> virtio_stw_p(vdev, _queues, s->conf.num_queues);
> -memcpy(config, , sizeof(struct virtio_blk_config));
> +memcpy(config, , VIRTIO_BLK_CFG_SIZE);
> +QEMU_BUILD_BUG_ON(VIRTIO_BLK_CFG_SIZE > sizeof(struct
> virtio_blk_config));
> +
Oh probably sizeof blkcfg here, right?
Also we don't need the empty line here.
> }
>
> static void virtio_blk_set_config(VirtIODevice *vdev, const uint8_t *config)
> @@ -769,7 +775,8 @@ static void virtio_blk_set_config(VirtIODevice *vdev,
> const uint8_t *config)
> VirtIOBlock *s = VIRTIO_BLK(vdev);
> struct virtio_blk_config blkcfg;
>
> -memcpy(, config, sizeof(blkcfg));
> +memcpy(, config, VIRTIO_BLK_CFG_SIZE);
> +QEMU_BUILD_BUG_ON(VIRTIO_BLK_CFG_SIZE > sizeof(blkcfg));
>
> aio_context_acquire(blk_get_aio_context(s->blk));
> blk_set_enable_write_cache(s->blk, blkcfg.wce != 0);
> @@ -952,8 +959,7 @@ static void virtio_blk_device_realize(DeviceState *dev,
> Error **errp)
> return;
> }
>
> -virtio_init(vdev, "virtio-blk", VIRTIO_ID_BLOCK,
> -sizeof(struct virtio_blk_config));
> +virtio_init(vdev, "virtio-blk", VIRTIO_ID_BLOCK, VIRTIO_BLK_CFG_SIZE);
>
> s->blk = conf->conf.blk;
> s->rq = NULL;
> --
> 1.9.3
[Qemu-devel] [PATCH 2/2] slirp: wrap the remaining socket functions
QEMU wraps the socket functions in os-win32.h, but in commit
a9d8b3ec4385793815d71217857304, the header inclusion was dropped,
breaking libslirp on Windows.
Wrap the missing functions.
Rename the wrapped function with "slirp_" prefix and "_wrap" suffix,
for consistency and to avoid a clash with existing function (such as
"slirp_socket").
Fixes: a9d8b3ec ("slirp: replace remaining qemu headers dependency")
Signed-off-by: Marc-André Lureau
---
slirp/util.h | 56 +++---
slirp/util.c | 164 ++-
2 files changed, 209 insertions(+), 11 deletions(-)
diff --git a/slirp/util.h b/slirp/util.h
index 685b5af099..c4207a49d6 100644
--- a/slirp/util.h
+++ b/slirp/util.h
@@ -83,23 +83,61 @@ struct iovec {
/* FIXME: remove me when made standalone */
#ifdef _WIN32
+#undef accept
+#undef bind
#undef closesocket
+#undef connect
+#undef getpeername
+#undef getsockname
#undef getsockopt
#undef ioctlsocket
+#undef listen
#undef recv
+#undef recvfrom
+#undef send
+#undef sendto
#undef setsockopt
+#undef shutdown
+#undef socket
#endif
#ifdef _WIN32
-#define closesocket slirp_closesocket
-int slirp_closesocket(int fd);
-#define ioctlsocket slirp_ioctlsocket
-int slirp_ioctlsocket(int fd, int req, void *val);
-#define getsockopt(sockfd, level, optname, optval, optlen) \
-getsockopt(sockfd, level, optname, (void *)optval, optlen)
-#define setsockopt(sockfd, level, optname, optval, optlen)\
-setsockopt(sockfd, level, optname, (const void *)optval, optlen)
-#define recv(sockfd, buf, len, flags) recv(sockfd, (void *)buf, len, flags)
+#define connect slirp_connect_wrap
+int slirp_connect_wrap(int fd, const struct sockaddr *addr, int addrlen);
+#define listen slirp_listen_wrap
+int slirp_listen_wrap(int fd, int backlog);
+#define bind slirp_bind_wrap
+int slirp_bind_wrap(int fd, const struct sockaddr *addr, int addrlen);
+#define socket slirp_socket_wrap
+int slirp_socket_wrap(int domain, int type, int protocol);
+#define accept slirp_accept_wrap
+int slirp_accept_wrap(int fd, struct sockaddr *addr, int *addrlen);
+#define shutdown slirp_shutdown_wrap
+int slirp_shutdown_wrap(int fd, int how);
+#define getpeername slirp_getpeername_wrap
+int slirp_getpeername_wrap(int fd, struct sockaddr *addr, int *addrlen);
+#define getsockname slirp_getsockname_wrap
+int slirp_getsockname_wrap(int fd, struct sockaddr *addr, int *addrlen);
+#define send slirp_send_wrap
+ssize_t slirp_send_wrap(int fd, const void *buf, size_t len, int flags);
+#define sendto slirp_sendto_wrap
+ssize_t slirp_sendto_wrap(int fd, const void *buf, size_t len, int flags,
+ const struct sockaddr *dest_addr, int addrlen);
+#define recv slirp_recv_wrap
+ssize_t slirp_recv_wrap(int fd, void *buf, size_t len, int flags);
+#define recvfrom slirp_recvfrom_wrap
+ssize_t slirp_recvfrom_wrap(int fd, void *buf, size_t len, int flags,
+struct sockaddr *src_addr, int *addrlen);
+#define closesocket slirp_closesocket_wrap
+int slirp_closesocket_wrap(int fd);
+#define ioctlsocket slirp_ioctlsocket_wrap
+int slirp_ioctlsocket_wrap(int fd, int req, void *val);
+#define getsockopt slirp_getsockopt_wrap
+int slirp_getsockopt_wrap(int sockfd, int level, int optname,
+ void *optval, int *optlen);
+#define setsockopt slirp_setsockopt_wrap
+int slirp_setsockopt_wrap(int sockfd, int level, int optname,
+ const void *optval, int optlen);
int inet_aton(const char *cp, struct in_addr *ia);
#else
diff --git a/slirp/util.c b/slirp/util.c
index 84f5afdbc3..1cbaa26b60 100644
--- a/slirp/util.c
+++ b/slirp/util.c
@@ -167,7 +167,7 @@ static int socket_error(void)
}
#undef ioctlsocket
-int slirp_ioctlsocket(int fd, int req, void *val)
+int slirp_ioctlsocket_wrap(int fd, int req, void *val)
{
int ret;
ret = ioctlsocket(fd, req, val);
@@ -178,7 +178,7 @@ int slirp_ioctlsocket(int fd, int req, void *val)
}
#undef closesocket
-int slirp_closesocket(int fd)
+int slirp_closesocket_wrap(int fd)
{
int ret;
ret = closesocket(fd);
@@ -187,6 +187,166 @@ int slirp_closesocket(int fd)
}
return ret;
}
+
+#undef connect
+int slirp_connect_wrap(int sockfd, const struct sockaddr *addr, int addrlen)
+{
+int ret;
+ret = connect(sockfd, addr, addrlen);
+if (ret < 0) {
+errno = socket_error();
+}
+return ret;
+}
+
+#undef listen
+int slirp_listen_wrap(int sockfd, int backlog)
+{
+int ret;
+ret = listen(sockfd, backlog);
+if (ret < 0) {
+errno = socket_error();
+}
+return ret;
+}
+
+#undef bind
+int slirp_bind_wrap(int sockfd, const struct sockaddr *addr, int addrlen)
+{
+int ret;
+ret = bind(sockfd, addr, addrlen);
+if (ret < 0) {
+errno = socket_error();
+}
+return ret;
+}
+
+#undef socket
+int slirp_socket_wrap(int domain, int type, int protocol)
+{
+int ret;
+ret = socket(domain, type, protocol);
+
[Qemu-devel] [PATCH 2/7] slirp: use libslirp migration code
slirp migration code uses QEMU vmstate so far, when building WITH_QEMU.
Introduce slirp_state_{load,save,version}() functions to move the
state saving handling to libslirp side.
So far, the bitstream compatibility should remain equal with current
QEMU, as this is effectively using the same code, with the same format
etc. When libslirp is made standalone, we will need some mechanism to
ensure bitstream compatibility regardless of the libslirp version
installed. See the FIXME note in the code.
Signed-off-by: Marc-André Lureau
---
include/migration/qemu-file-types.h | 2 ++
migration/qemu-file.h | 1 -
slirp/libslirp.h| 8 +
slirp/state.h | 9 -
net/slirp.c | 55 +
slirp/slirp.c | 9 -
slirp/state.c | 52 ---
7 files changed, 88 insertions(+), 48 deletions(-)
diff --git a/include/migration/qemu-file-types.h
b/include/migration/qemu-file-types.h
index bd6d7dd7f9..bbe04d4484 100644
--- a/include/migration/qemu-file-types.h
+++ b/include/migration/qemu-file-types.h
@@ -25,6 +25,8 @@
#ifndef QEMU_FILE_H
#define QEMU_FILE_H
+int qemu_file_get_error(QEMUFile *f);
+
void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, size_t size);
void qemu_put_byte(QEMUFile *f, int v);
diff --git a/migration/qemu-file.h b/migration/qemu-file.h
index 2ccfcfb2a8..13baf896bd 100644
--- a/migration/qemu-file.h
+++ b/migration/qemu-file.h
@@ -149,7 +149,6 @@ void qemu_update_position(QEMUFile *f, size_t size);
void qemu_file_reset_rate_limit(QEMUFile *f);
void qemu_file_set_rate_limit(QEMUFile *f, int64_t new_rate);
int64_t qemu_file_get_rate_limit(QEMUFile *f);
-int qemu_file_get_error(QEMUFile *f);
void qemu_file_set_error(QEMUFile *f, int ret);
int qemu_file_shutdown(QEMUFile *f);
QEMUFile *qemu_file_get_return_path(QEMUFile *f);
diff --git a/slirp/libslirp.h b/slirp/libslirp.h
index 18c50bd7ba..7ce1e4d0d4 100644
--- a/slirp/libslirp.h
+++ b/slirp/libslirp.h
@@ -101,6 +101,14 @@ void slirp_socket_recv(Slirp *slirp, struct in_addr
guest_addr,
int guest_port, const uint8_t *buf, int size);
size_t slirp_socket_can_recv(Slirp *slirp, struct in_addr guest_addr,
int guest_port);
+
+void slirp_state_save(Slirp *s, SlirpWriteCb write_cb, void *opaque);
+
+int slirp_state_load(Slirp *s, int version_id,
+ SlirpReadCb read_cb, void *opaque);
+
+int slirp_state_version(void);
+
#ifdef __cplusplus
} /* extern "C" */
#endif
diff --git a/slirp/state.h b/slirp/state.h
index 154866898f..e69de29bb2 100644
--- a/slirp/state.h
+++ b/slirp/state.h
@@ -1,9 +0,0 @@
-#ifndef SLIRP_STATE_H_
-#define SLIRP_STATE_H_
-
-#include "libslirp.h"
-
-void slirp_state_register(Slirp *slirp);
-void slirp_state_unregister(Slirp *slirp);
-
-#endif /* SLIRP_STATE_H_ */
diff --git a/net/slirp.c b/net/slirp.c
index 7a16d8d615..bcd91475d2 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -44,6 +44,8 @@
#include "qapi/error.h"
#include "qapi/qmp/qdict.h"
#include "util.h"
+#include "migration/register.h"
+#include "migration/qemu-file-types.h"
static int get_str_sep(char *buf, int buf_size, const char **pp, int sep)
{
@@ -146,6 +148,7 @@ static void net_slirp_cleanup(NetClientState *nc)
g_slist_free_full(s->fwd, slirp_free_fwd);
main_loop_poll_remove_notifier(>poll_notifier);
+unregister_savevm(NULL, "slirp", s->slirp);
slirp_cleanup(s->slirp);
if (s->exit_notifier.notify) {
qemu_remove_exit_notifier(>exit_notifier);
@@ -303,6 +306,46 @@ static void net_slirp_poll_notify(Notifier *notifier, void
*data)
}
}
+static ssize_t
+net_slirp_stream_read(void *buf, size_t size, void *opaque)
+{
+QEMUFile *f = opaque;
+
+return qemu_get_buffer(f, buf, size);
+}
+
+static ssize_t
+net_slirp_stream_write(const void *buf, size_t size, void *opaque)
+{
+QEMUFile *f = opaque;
+
+qemu_put_buffer(f, buf, size);
+if (qemu_file_get_error(f)) {
+return -1;
+}
+
+return size;
+}
+
+static int net_slirp_state_load(QEMUFile *f, void *opaque, int version_id)
+{
+Slirp *slirp = opaque;
+
+return slirp_state_load(slirp, version_id, net_slirp_stream_read, f);
+}
+
+static void net_slirp_state_save(QEMUFile *f, void *opaque)
+{
+Slirp *slirp = opaque;
+
+slirp_state_save(slirp, net_slirp_stream_write, f);
+}
+
+static SaveVMHandlers savevm_slirp_state = {
+.save_state = net_slirp_state_save,
+.load_state = net_slirp_state_load,
+};
+
static int net_slirp_init(NetClientState *peer, const char *model,
const char *name, int restricted,
bool ipv4, const char *vnetwork, const char *vhost,
@@ -523,6 +566,18 @@ static int net_slirp_init(NetClientState *peer, const char
*model,
_cb, s);
QTAILQ_INSERT_TAIL(_stacks, s, entry);
+
Re: [Qemu-devel] [PATCH v2] virtio-blk: set correct config size for the host driver
On Tue, 12 Feb 2019 14:01:44 +0800
Changpeng Liu wrote:
> Commit caa1ee43 "vhost-user-blk: add discard/write zeroes features support"
> introduced extra fields to existing struct virtio_blk_config, when
> migration was executed from older QEMU version to current head, it
A hint about the breakage is often a good practice, as it helps
people to find the fix if they hit the issue.
> will break the migration. While here, set the correct config size
"While here" usually means that the corresponding change isn't strictly
needed. I don't believe this is the case here since fixing the config
size is precisely the goal of this patch.
> when initializing the host driver, for now, discard/write zeroes
> are not supported by virtio-blk host driver, so set the config
As mentioned by Michael, there's no such thing as a "host driver".
Just say virtio-blk.
> size as before, users can change config size when adding the new
> feature bits support.
>
What about ?
Commit caa1ee43 "vhost-user-blk: add discard/write zeroes features
support" added fields to struct virtio_blk_config. This changes
the size of the config space and breaks migration from QEMU 3.1
and older:
qemu-system-ppc64: get_pci_config_device: Bad config data: i=0x10 read: 41
device: 1 cmask: ff wmask: 80 w1cmask:0
qemu-system-ppc64: Failed to load PCIDevice:config
qemu-system-ppc64: Failed to load virtio-blk:virtio
qemu-system-ppc64: error while loading state for instance 0x0 of device
'pci@8002000:01.0/virtio-blk'
qemu-system-ppc64: load of migration failed: Invalid argument
Since virtio-blk doesn't support the "discard" and "write zeroes"
features, it shouldn't even expose the associated fields in the
config space actually. Just include all fields up to num_queues to
match QEMU 3.1 and older.
> Signed-off-by: Changpeng Liu
> ---
LGTM
Reviewed-by: Greg Kurz
Tested-by: Greg Kurz
> hw/block/virtio-blk.c | 10 ++
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
> index 9a87b3b..846b7b9 100644
> --- a/hw/block/virtio-blk.c
> +++ b/hw/block/virtio-blk.c
> @@ -28,6 +28,9 @@
> #include "hw/virtio/virtio-bus.h"
> #include "hw/virtio/virtio-access.h"
>
> +#define VIRTIO_BLK_CFG_SIZE (offsetof(struct virtio_blk_config, num_queues)
> + \
> + sizeof_field(struct virtio_blk_config,
> num_queues))
> +
> static void virtio_blk_init_request(VirtIOBlock *s, VirtQueue *vq,
> VirtIOBlockReq *req)
> {
> @@ -761,7 +764,7 @@ static void virtio_blk_update_config(VirtIODevice *vdev,
> uint8_t *config)
> blkcfg.alignment_offset = 0;
> blkcfg.wce = blk_enable_write_cache(s->blk);
> virtio_stw_p(vdev, _queues, s->conf.num_queues);
> -memcpy(config, , sizeof(struct virtio_blk_config));
> +memcpy(config, , VIRTIO_BLK_CFG_SIZE);
> }
>
> static void virtio_blk_set_config(VirtIODevice *vdev, const uint8_t *config)
> @@ -769,7 +772,7 @@ static void virtio_blk_set_config(VirtIODevice *vdev,
> const uint8_t *config)
> VirtIOBlock *s = VIRTIO_BLK(vdev);
> struct virtio_blk_config blkcfg;
>
> -memcpy(, config, sizeof(blkcfg));
> +memcpy(, config, VIRTIO_BLK_CFG_SIZE);
>
> aio_context_acquire(blk_get_aio_context(s->blk));
> blk_set_enable_write_cache(s->blk, blkcfg.wce != 0);
> @@ -952,8 +955,7 @@ static void virtio_blk_device_realize(DeviceState *dev,
> Error **errp)
> return;
> }
>
> -virtio_init(vdev, "virtio-blk", VIRTIO_ID_BLOCK,
> -sizeof(struct virtio_blk_config));
> +virtio_init(vdev, "virtio-blk", VIRTIO_ID_BLOCK, VIRTIO_BLK_CFG_SIZE);
>
> s->blk = conf->conf.blk;
> s->rq = NULL;
[Qemu-devel] [PATCH 4/4] wavcapture: Convert to error_report
From: "Dr. David Alan Gilbert"
Kill off a pile of monitor_printf's and cur_mon usage.
The only one left in wavcapture.c is the info case.
Signed-off-by: Dr. David Alan Gilbert
---
audio/wavcapture.c | 39 +--
1 file changed, 17 insertions(+), 22 deletions(-)
diff --git a/audio/wavcapture.c b/audio/wavcapture.c
index cf31ed652c..cd24570aa7 100644
--- a/audio/wavcapture.c
+++ b/audio/wavcapture.c
@@ -38,30 +38,29 @@ static void wav_destroy (void *opaque)
uint8_t dlen[4];
uint32_t datalen = wav->bytes;
uint32_t rifflen = datalen + 36;
-Monitor *mon = cur_mon;
if (wav->f) {
le_store (rlen, rifflen, 4);
le_store (dlen, datalen, 4);
if (fseek (wav->f, 4, SEEK_SET)) {
-monitor_printf (mon, "wav_destroy: rlen fseek failed\nReason:
%s\n",
-strerror (errno));
+error_report("wav_destroy: rlen fseek failed: %s",
+ strerror(errno));
goto doclose;
}
if (fwrite (rlen, 4, 1, wav->f) != 1) {
-monitor_printf (mon, "wav_destroy: rlen fwrite failed\nReason
%s\n",
-strerror (errno));
+error_report("wav_destroy: rlen fwrite failed: %s",
+ strerror(errno));
goto doclose;
}
if (fseek (wav->f, 32, SEEK_CUR)) {
-monitor_printf (mon, "wav_destroy: dlen fseek failed\nReason %s\n",
-strerror (errno));
+error_report("wav_destroy: dlen fseek failed: %s",
+ strerror(errno));
goto doclose;
}
if (fwrite (dlen, 1, 4, wav->f) != 4) {
-monitor_printf (mon, "wav_destroy: dlen fwrite failed\nReason
%s\n",
-strerror (errno));
+error_report("wav_destroy: dlen fwrite failed: %s",
+ strerror(errno));
goto doclose;
}
doclose:
@@ -78,8 +77,7 @@ static void wav_capture (void *opaque, void *buf, int size)
WAVState *wav = opaque;
if (fwrite (buf, size, 1, wav->f) != 1) {
-monitor_printf (cur_mon, "wav_capture: fwrite error\nReason: %s",
-strerror (errno));
+error_report("wav_capture: fwrite error: %s", strerror(errno));
}
wav->bytes += size;
}
@@ -110,7 +108,6 @@ static struct capture_ops wav_capture_ops = {
int wav_start_capture (CaptureState *s, const char *path, int freq,
int bits, int nchannels)
{
-Monitor *mon = cur_mon;
WAVState *wav;
uint8_t hdr[] = {
0x52, 0x49, 0x46, 0x46, 0x00, 0x00, 0x00, 0x00, 0x57, 0x41, 0x56,
@@ -124,13 +121,13 @@ int wav_start_capture (CaptureState *s, const char *path,
int freq,
CaptureVoiceOut *cap;
if (bits != 8 && bits != 16) {
-monitor_printf (mon, "incorrect bit count %d, must be 8 or 16\n",
bits);
+error_report("incorrect bit count %d, must be 8 or 16", bits);
return -1;
}
if (nchannels != 1 && nchannels != 2) {
-monitor_printf (mon, "incorrect channel count %d, must be 1 or 2\n",
-nchannels);
+error_report("incorrect channel count %d, must be 1 or 2",
+ nchannels);
return -1;
}
@@ -158,8 +155,8 @@ int wav_start_capture (CaptureState *s, const char *path,
int freq,
wav->f = fopen (path, "wb");
if (!wav->f) {
-monitor_printf (mon, "Failed to open wave file `%s'\nReason: %s\n",
-path, strerror (errno));
+error_report("Failed to open wave file `%s': %s",
+ path, strerror(errno));
g_free (wav);
return -1;
}
@@ -170,14 +167,13 @@ int wav_start_capture (CaptureState *s, const char *path,
int freq,
wav->freq = freq;
if (fwrite (hdr, sizeof (hdr), 1, wav->f) != 1) {
-monitor_printf (mon, "Failed to write header\nReason: %s\n",
-strerror (errno));
+error_report("Failed to write header: %s", strerror(errno));
goto error_free;
}
cap = AUD_add_capture (, , wav);
if (!cap) {
-monitor_printf (mon, "Failed to add audio capture\n");
+error_report("Failed to add audio capture");
goto error_free;
}
@@ -189,8 +185,7 @@ int wav_start_capture (CaptureState *s, const char *path,
int freq,
error_free:
g_free (wav->path);
if (fclose (wav->f)) {
-monitor_printf (mon, "Failed to close wave file\nReason: %s\n",
-strerror (errno));
+error_report("Failed to close wave file: %s", strerror(errno));
}
g_free (wav);
return -1;
--
2.20.1
Re: [Qemu-devel] [PATCH] hostmem-file: reject invalid pmem file sizes
On Tue, 12 Feb 2019 10:52:41 +0800
Stefan Hajnoczi wrote:
> Guests started with NVDIMMs larger than the underlying host file produce
> confusing errors inside the guest. This happens because the guest
> accesses pages beyond the end of the file.
>
> Check the pmem file size on startup and print a clear error message if
> the size is invalid.
>
> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1669053
> Cc: Haozhong Zhang
> Cc: Zhang Yi
> Cc: Eduardo Habkost
> Cc: Igor Mammedov
> Signed-off-by: Stefan Hajnoczi
> ---
> include/qemu/osdep.h| 13 ++
> backends/hostmem-file.c | 16 +
> util/oslib-posix.c | 53 +
> util/oslib-win32.c | 5
> 4 files changed, 87 insertions(+)
>
> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> index 840af09cb0..303d315c5d 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -570,6 +570,19 @@ void qemu_set_tty_echo(int fd, bool echo);
> void os_mem_prealloc(int fd, char *area, size_t sz, int smp_cpus,
> Error **errp);
>
> +/**
> + * qemu_get_pmem_size:
> + * @filename: path to a pmem file
> + * @errp: pointer to a NULL-initialized error object
> + *
> + * Determine the size of a persistent memory file. Besides supporting files
> on
> + * DAX file systems, this function also supports Linux devdax character
> + * devices.
> + *
> + * Returns: the size or 0 on failure
> + */
> +uint64_t qemu_get_pmem_size(const char *filename, Error **errp);
> +
> /**
> * qemu_get_pid_name:
> * @pid: pid of a process
> diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
> index ba601ce940..325ab4aad9 100644
> --- a/backends/hostmem-file.c
> +++ b/backends/hostmem-file.c
> @@ -46,6 +46,22 @@ file_backend_memory_alloc(HostMemoryBackend *backend,
> Error **errp)
> gchar *name;
> #endif
>
> +/*
> + * Verify pmem file size since starting a guest with an incorrect size
> + * leads to confusing failures inside the guest.
> + */
> +if (fb->is_pmem && fb->mem_path) {
> +uint64_t size;
> +
> +size = qemu_get_pmem_size(fb->mem_path, NULL);
Did you ignore error intentionally?
> +if (size && backend->size > size) {
> +error_setg(errp, "size property %" PRIu64 " is larger than "
> + "pmem file \"%s\" size %" PRIu64, backend->size,
> + fb->mem_path, size);
> +return;
> +}
> +}
> +
> if (!backend->size) {
> error_setg(errp, "can't create backend with size 0");
> return;
> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> index 37c5854b9c..10d90d1783 100644
> --- a/util/oslib-posix.c
> +++ b/util/oslib-posix.c
> @@ -500,6 +500,59 @@ void os_mem_prealloc(int fd, char *area, size_t memory,
> int smp_cpus,
> }
> }
>
> +uint64_t qemu_get_pmem_size(const char *filename, Error **errp)
> +{
> +struct stat st;
> +
> +if (stat(filename, ) < 0) {
> +error_setg(errp, "unable to stat pmem file \"%s\"", filename);
> +return 0;
> +}
> +
> +#if defined(__linux__)
> +/* Special handling for devdax character devices */
> +if (S_ISCHR(st.st_mode)) {
> +char *subsystem_path = NULL;
> +char *subsystem = NULL;
> +char *size_path = NULL;
> +char *size_str = NULL;
> +uint64_t ret = 0;
> +
> +subsystem_path = g_strdup_printf("/sys/dev/char/%d:%d/subsystem",
> + major(st.st_rdev),
> minor(st.st_rdev));
> +subsystem = g_file_read_link(subsystem_path, NULL);
> +if (!subsystem) {
> +error_setg(errp, "unable to read subsystem for pmem file \"%s\"",
> + filename);
> +goto devdax_err;
> +}
> +
> +if (!g_str_has_suffix(subsystem, "/dax")) {
> +error_setg(errp, "pmem file \"%s\" is not a dax device",
> filename);
> +goto devdax_err;
> +}
> +
> +size_path = g_strdup_printf("/sys/dev/char/%d:%d/size",
> +major(st.st_rdev), minor(st.st_rdev));
> +if (!g_file_get_contents(size_path, _str, NULL, NULL)) {
> +error_setg(errp, "unable to read size for pmem file \"%s\"",
> + size_path);
> +goto devdax_err;
> +}
> +
> +ret = g_ascii_strtoull(size_str, NULL, 0);
> +
> +devdax_err:
> +g_free(size_str);
> +g_free(size_path);
> +g_free(subsystem);
> +g_free(subsystem_path);
> +return ret;
> +}
> +#endif /* defined(__linux__) */
> +
> +return st.st_size;
> +}
>
> char *qemu_get_pid_name(pid_t pid)
> {
> diff --git a/util/oslib-win32.c b/util/oslib-win32.c
> index b4c17f5dfa..bd633afab6 100644
> --- a/util/oslib-win32.c
> +++ b/util/oslib-win32.c
> @@ -560,6 +560,11 @@ void
Re: [Qemu-devel] [PATCH] s390x/kvm: add tracepoint to ioeventfd interface
On 12.02.19 16:58, Cornelia Huck wrote:
> On Tue, 12 Feb 2019 16:52:09 +0100
> Philippe Mathieu-Daudé wrote:
>
>> Hi Cornelia,
>>
>> On 2/12/19 4:30 PM, Cornelia Huck wrote:
>>> Trace when assigning/unassigning.
>>>
>>> Signed-off-by: Cornelia Huck
>>> ---
>>> target/s390x/kvm.c| 2 ++
>>> target/s390x/trace-events | 1 +
>>> 2 files changed, 3 insertions(+)
>>>
>>> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
>>> index 15fdc168e1c5..7d61bd109092 100644
>>> --- a/target/s390x/kvm.c
>>> +++ b/target/s390x/kvm.c
>>> @@ -1886,6 +1886,8 @@ int kvm_s390_assign_subch_ioeventfd(EventNotifier
>>> *notifier, uint32_t sch,
>>> .addr = sch,
>>> .len = 8,
>>> };
>>> +trace_kvm_s390_assign_subch_ioeventfd(kick.fd, kick.addr, assign,
>>> + kick.datamatch);
>>> if (!kvm_check_extension(kvm_state, KVM_CAP_IOEVENTFD)) {
>>> return -ENOSYS;
>>> }
>>> diff --git a/target/s390x/trace-events b/target/s390x/trace-events
>>> index a84e316e4937..bdc22f42cdbb 100644
>>> --- a/target/s390x/trace-events
>>> +++ b/target/s390x/trace-events
>>> @@ -14,6 +14,7 @@ ioinst_chsc_cmd(uint16_t cmd, uint16_t len) "IOINST: chsc
>>> command 0x%04x, len 0x
>>> kvm_enable_cmma(int rc) "CMMA: enabling with result code %d"
>>> kvm_clear_cmma(int rc) "CMMA: clearing with result code %d"
>>> kvm_failed_cpu_state_set(int cpu_index, uint8_t state, const char *msg)
>>> "Warning: Unable to set cpu %d state %" PRIu8 " to KVM: %s"
>>> +kvm_s390_assign_subch_ioeventfd(int fd, uint32_t addr, bool assign, int
>>> datamatch) "fd: %d sch: @0x%x assign: %d vq: %d"
>>
>> I noticed all s390x related trace events don't specify 's390' in the
>> event name, maybe you can simply strip it. If you agree, feel free to
>> apply that change directly yourself, no need for v2 ;)
>
> Yeah; not exactly sure why the other events do that, though... I can
> strip it if others agree.
Yes, let's keep it consistent. Or if you feel like crafting patches,
rename the others ;)
>
>>
>> Regardless the name used:
>> Reviewed-by: Philippe Mathieu-Daudé
>
> Thanks!
>
>>
>>>
>>> # target/s390x/cpu.c
>>> cpu_set_state(int cpu_index, uint8_t state) "setting cpu %d state to %"
>>> PRIu8
>>>
>
--
Thanks,
David / dhildenb
[Qemu-devel] [PATCH 1/7] slirp: adapt a subset of QEMU vmstate code
Add vmstate serialization code adapted from QEMU.
Keep only the bits that are required for libslirp.
Introduce a IStream/OStream interface to replace QEMU QFile
abstraction.
Signed-off-by: Marc-André Lureau
---
slirp/libslirp.h| 1 +
slirp/stream.h | 34
slirp/vmstate.h | 396 +++
slirp/stream.c | 119 +
slirp/vmstate.c | 401
slirp/Makefile.objs | 2 +
6 files changed, 953 insertions(+)
create mode 100644 slirp/stream.h
create mode 100644 slirp/vmstate.h
create mode 100644 slirp/stream.c
create mode 100644 slirp/vmstate.c
diff --git a/slirp/libslirp.h b/slirp/libslirp.h
index fccab42518..18c50bd7ba 100644
--- a/slirp/libslirp.h
+++ b/slirp/libslirp.h
@@ -26,6 +26,7 @@ enum {
SLIRP_POLL_HUP = 1 << 4,
};
+typedef ssize_t (*SlirpReadCb)(void *buf, size_t len, void *opaque);
typedef ssize_t (*SlirpWriteCb)(const void *buf, size_t len, void *opaque);
typedef void (*SlirpTimerCb)(void *opaque);
typedef int (*SlirpAddPollCb)(int fd, int events, void *opaque);
diff --git a/slirp/stream.h b/slirp/stream.h
new file mode 100644
index 00..985334c043
--- /dev/null
+++ b/slirp/stream.h
@@ -0,0 +1,34 @@
+#ifndef STREAM_H_
+#define STREAM_H_
+
+#include "libslirp.h"
+
+typedef struct SlirpIStream {
+SlirpReadCb read_cb;
+void *opaque;
+} SlirpIStream;
+
+typedef struct SlirpOStream {
+SlirpWriteCb write_cb;
+void *opaque;
+} SlirpOStream;
+
+bool slirp_istream_read(SlirpIStream *f, void *buf, size_t size);
+bool slirp_ostream_write(SlirpOStream *f, const void *buf, size_t size);
+
+uint8_t slirp_istream_read_u8(SlirpIStream *f);
+bool slirp_ostream_write_u8(SlirpOStream *f, uint8_t b);
+
+uint16_t slirp_istream_read_u16(SlirpIStream *f);
+bool slirp_ostream_write_u16(SlirpOStream *f, uint16_t b);
+
+uint32_t slirp_istream_read_u32(SlirpIStream *f);
+bool slirp_ostream_write_u32(SlirpOStream *f, uint32_t b);
+
+int16_t slirp_istream_read_i16(SlirpIStream *f);
+bool slirp_ostream_write_i16(SlirpOStream *f, int16_t b);
+
+int32_t slirp_istream_read_i32(SlirpIStream *f);
+bool slirp_ostream_write_i32(SlirpOStream *f, int32_t b);
+
+#endif /* STREAM_H_ */
diff --git a/slirp/vmstate.h b/slirp/vmstate.h
new file mode 100644
index 00..6ffad27de9
--- /dev/null
+++ b/slirp/vmstate.h
@@ -0,0 +1,396 @@
+/*
+ * QEMU migration/snapshot declarations
+ *
+ * Copyright (c) 2009-2011 Red Hat, Inc.
+ *
+ * Original author: Juan Quintela
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+#ifndef VMSTATE_H_
+#define VMSTATE_H_
+
+#include
+#include
+#include
+#include "slirp.h"
+#include "stream.h"
+
+#define stringify(s) tostring(s)
+#define tostring(s) #s
+
+typedef struct VMStateInfo VMStateInfo;
+typedef struct VMStateDescription VMStateDescription;
+typedef struct VMStateField VMStateField;
+
+int slirp_vmstate_save_state(SlirpOStream *f, const VMStateDescription *vmsd,
+ void *opaque);
+int slirp_vmstate_load_state(SlirpIStream *f, const VMStateDescription *vmsd,
+ void *opaque, int version_id);
+
+/* VMStateInfo allows customized migration of objects that don't fit in
+ * any category in VMStateFlags. Additional information is always passed
+ * into get and put in terms of field and vmdesc parameters. However
+ * these two parameters should only be used in cases when customized
+ * handling is needed, such as QTAILQ. For primitive data types such as
+ * integer, field and vmdesc parameters should be ignored inside get/put.
+ */
+struct VMStateInfo {
+const char *name;
+int (*get)(SlirpIStream *f, void *pv, size_t size, VMStateField *field);
+int (*put)(SlirpOStream *f, void *pv, size_t size, VMStateField *field);
+};
+
+enum VMStateFlags {
+/* Ignored */
+VMS_SINGLE = 0x001,
+
+/* The struct member at opaque + VMStateField.offset is
[Qemu-devel] [PATCH] linux-user: fix recvmsg emulation
Set msg_flags in the returned struct msghdr.
Signed-off-by: Andreas Schwab
---
linux-user/syscall.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 90bfda3563..b6b566a6fa 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -2797,6 +2797,7 @@ static abi_long do_sendrecvmsg_locked(int fd, struct
target_msghdr *msgp,
}
if (!is_error(ret)) {
msgp->msg_namelen = tswap32(msg.msg_namelen);
+msgp->msg_flags = tswap32(msg.msg_flags);
if (msg.msg_name != NULL && msg.msg_name != (void *)-1) {
ret = host_to_target_sockaddr(tswapal(msgp->msg_name),
msg.msg_name, msg.msg_namelen);
--
2.20.1
--
Andreas Schwab, SUSE Labs, sch...@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
[Qemu-devel] Key repeat is no longer working on TTY and grub menu
I noticed that the key inputs are no longer repeating when using default setting on QEMU. For example, if I press "a" and keep it pressed it will print only one "a" instead of repeating them. Another example is when deleting text: instead of holding backspace pressed and the text is deleted, it's needed to press backspace once for each character. As this makes many tasks much slower, requiring many key presses for tasks that were done in a single press, I did a bisect to see when this change happened. The result is: 0c0d42737dfdcea872a987ecba6ef55047df8881 is the first bad commit commit 0c0d42737dfdcea872a987ecba6ef55047df8881 Author: Gerd Hoffmann Date: Tue Jan 22 10:28:11 2019 +0100 kbd-state: use state tracker for gtk Use the new keyboard state tracked for gtk. Allows to drop the gtk-specific modifier state tracking code. Signed-off-by: Gerd Hoffmann Reviewed-by: Daniel P. Berrangé Message-id: 20190122092814.14919-6-kra...@redhat.com :04 04 e882f144bafca90257a4604b5e62486a1ca038b2 0139afe1aee24256c3f711c080b0230ab104074a M include :04 04 c258efc06107b944b0627792b9f2377600033118 4fd01a25a3e0d1fccca72c35ec58b205aa4fd882 M ui I'm using the GTK interface and all guests without a Graphical User Interface (GUI) are affected by this. This happens on grub menu and on TTY of multiple Linux guests. Workarounds found for the key repeat issue: -Installing a GUI to server systems; -Adding a usb-kbd device, but then the key '/' is lost.
[Qemu-devel] [PATCH 1/4] pckbd: Convert DPRINTF->trace
From: "Dr. David Alan Gilbert"
Signed-off-by: Dr. David Alan Gilbert
---
hw/input/pckbd.c | 19 ++-
hw/input/trace-events | 7 +++
2 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/hw/input/pckbd.c b/hw/input/pckbd.c
index 72e7d5f6cc..47a606f5e3 100644
--- a/hw/input/pckbd.c
+++ b/hw/input/pckbd.c
@@ -30,14 +30,7 @@
#include "hw/input/i8042.h"
#include "sysemu/sysemu.h"
-/* debug PC keyboard */
-//#define DEBUG_KBD
-#ifdef DEBUG_KBD
-#define DPRINTF(fmt, ...) \
-do { printf("KBD: " fmt , ## __VA_ARGS__); } while (0)
-#else
-#define DPRINTF(fmt, ...)
-#endif
+#include "trace.h"
/* Keyboard Controller Commands */
#define KBD_CCMD_READ_MODE 0x20/* Read mode bits */
@@ -210,7 +203,7 @@ static uint64_t kbd_read_status(void *opaque, hwaddr addr,
KBDState *s = opaque;
int val;
val = s->status;
-DPRINTF("kbd: read status=0x%02x\n", val);
+trace_pckbd_kbd_read_status(val);
return val;
}
@@ -224,7 +217,7 @@ static void kbd_queue(KBDState *s, int b, int aux)
static void outport_write(KBDState *s, uint32_t val)
{
-DPRINTF("kbd: write outport=0x%02x\n", val);
+trace_pckbd_outport_write(val);
s->outport = val;
qemu_set_irq(s->a20_out, (val >> 1) & 1);
if (!(val & 1)) {
@@ -237,7 +230,7 @@ static void kbd_write_command(void *opaque, hwaddr addr,
{
KBDState *s = opaque;
-DPRINTF("kbd: write cmd=0x%02" PRIx64 "\n", val);
+trace_pckbd_kbd_write_command(val);
/* Bits 3-0 of the output port P2 of the keyboard controller may be pulsed
* low for approximately 6 micro seconds. Bits 3-0 of the KBD_CCMD_PULSE
@@ -326,7 +319,7 @@ static uint64_t kbd_read_data(void *opaque, hwaddr addr,
else
val = ps2_read_data(s->kbd);
-DPRINTF("kbd: read data=0x%02x\n", val);
+trace_pckbd_kbd_read_data(val);
return val;
}
@@ -335,7 +328,7 @@ static void kbd_write_data(void *opaque, hwaddr addr,
{
KBDState *s = opaque;
-DPRINTF("kbd: write data=0x%02" PRIx64 "\n", val);
+trace_pckbd_kbd_write_data(val);
switch(s->write_cmd) {
case 0:
diff --git a/hw/input/trace-events b/hw/input/trace-events
index 3965a842ae..8e53ae5bbf 100644
--- a/hw/input/trace-events
+++ b/hw/input/trace-events
@@ -14,6 +14,13 @@ adb_mouse_readreg(int reg, uint8_t val0, uint8_t val1) "reg
%d obuf[0] 0x%2.2x o
adb_mouse_request_change_addr(int devaddr) "change addr to 0x%x"
adb_mouse_request_change_addr_and_handler(int devaddr, int handler) "change
addr and handler to 0x%x, 0x%x"
+# hw/input/pckbd.c
+pckbd_kbd_read_data(uint32_t val) "0x%02x"
+pckbd_kbd_read_status(int status) "0x%02x"
+pckbd_outport_write(uint32_t val) "0x%02x"
+pckbd_kbd_write_command(uint64_t val) "0x%02"PRIx64
+pckbd_kbd_write_data(uint64_t val) "0x%02"PRIx64
+
# hw/input/ps2.c
ps2_put_keycode(void *opaque, int keycode) "%p keycode 0x%02x"
ps2_keyboard_event(void *opaque, int qcode, int down, unsigned int modifier,
unsigned int modifiers) "%p qcode %d down %d modifier 0x%x modifiers 0x%x"
--
2.20.1
[Qemu-devel] [PATCH 3/4] kvm: Add kvm_set_ioeventfd* traces
From: "Dr. David Alan Gilbert"
Add a couple of traces around the kvm_set_ioeventfd* calls.
Signed-off-by: Dr. David Alan Gilbert
---
accel/kvm/kvm-all.c| 3 +++
accel/kvm/trace-events | 2 ++
2 files changed, 5 insertions(+)
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 4e1de942ce..fd92b6f375 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -657,6 +657,8 @@ static int kvm_set_ioeventfd_mmio(int fd, hwaddr addr,
uint32_t val,
.fd = fd,
};
+trace_kvm_set_ioeventfd_mmio(fd, (uint64_t)addr, val, assign, size,
+ datamatch);
if (!kvm_enabled()) {
return -ENOSYS;
}
@@ -688,6 +690,7 @@ static int kvm_set_ioeventfd_pio(int fd, uint16_t addr,
uint16_t val,
.fd = fd,
};
int r;
+trace_kvm_set_ioeventfd_pio(fd, addr, val, assign, size, datamatch);
if (!kvm_enabled()) {
return -ENOSYS;
}
diff --git a/accel/kvm/trace-events b/accel/kvm/trace-events
index 58e98efe5d..8841025d68 100644
--- a/accel/kvm/trace-events
+++ b/accel/kvm/trace-events
@@ -12,5 +12,7 @@ kvm_irqchip_commit_routes(void) ""
kvm_irqchip_add_msi_route(char *name, int vector, int virq) "dev %s vector %d
virq %d"
kvm_irqchip_update_msi_route(int virq) "Updating MSI route virq=%d"
kvm_irqchip_release_virq(int virq) "virq %d"
+kvm_set_ioeventfd_mmio(int fd, uint64_t addr, uint32_t val, bool assign,
uint32_t size, bool datamatch) "fd: %d @0x%" PRIx64 " val=0x%x assign: %d size:
%d match: %d"
+kvm_set_ioeventfd_pio(int fd, uint16_t addr, uint32_t val, bool assign,
uint32_t size, bool datamatch) "fd: %d @0x%x val=0x%x assign: %d size: %d
match: %d"
kvm_set_user_memory(uint32_t slot, uint32_t flags, uint64_t guest_phys_addr,
uint64_t memory_size, uint64_t userspace_addr, int ret) "Slot#%d flags=0x%x
gpa=0x%"PRIx64 " size=0x%"PRIx64 " ua=0x%"PRIx64 " ret=%d"
--
2.20.1
[Qemu-devel] [PATCH 0/4] a few trivials
From: "Dr. David Alan Gilbert" Just cleaning out my trivial pile; a few more traces and a couple of error reporting tweaks. Dave Dr. David Alan Gilbert (4): pckbd: Convert DPRINTF->trace HMP: Prepend errors with 'Error:' kvm: Add kvm_set_ioeventfd* traces wavcapture: Convert to error_report accel/kvm/kvm-all.c| 3 +++ accel/kvm/trace-events | 2 ++ audio/wavcapture.c | 39 +-- hmp.c | 2 +- hw/input/pckbd.c | 19 ++- hw/input/trace-events | 7 +++ 6 files changed, 36 insertions(+), 36 deletions(-) -- 2.20.1
Re: [Qemu-devel] [PATCH 2/4] HMP: Prepend errors with 'Error:'
On 2/12/19 2:47 PM, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert"
>
> Always make error messages start with 'Error:' as a fallback
> to make sure that anything parsing them can tell it failed.
>
> Note: Some places don't use hmp_handle_error
>
> Signed-off-by: Dr. David Alan Gilbert
Reviewed-by: Philippe Mathieu-Daudé
> ---
> hmp.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hmp.c b/hmp.c
> index b2a2b1f84e..4d14718fea 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -62,7 +62,7 @@ static void hmp_handle_error(Monitor *mon, Error **errp)
> {
> assert(errp);
> if (*errp) {
> -error_report_err(*errp);
> +error_reportf_err(*errp, "Error: ");
> }
> }
>
>
[Qemu-devel] [RFC PULL 2/6] virtio-blk: add host_features field in VirtIOBlock
From: Stefano Garzarella
Since configurable features for virtio-blk are growing, this patch
adds host_features field in the struct VirtIOBlock. (as in virtio-net)
In this way, we can avoid to add new fields for new properties and
we can directly set VIRTIO_BLK_F* flags in the host_features.
We update "config-wce" and "scsi" property definition to use the new
host_features field without change the behaviour.
Suggested-by: Michael S. Tsirkin
Reviewed-by: Stefan Hajnoczi
Signed-off-by: Stefano Garzarella
Acked-by: Pankaj Gupta
Message-id: 20190208134950.187665-3-sgarz...@redhat.com
Signed-off-by: Stefan Hajnoczi
---
include/hw/virtio/virtio-blk.h | 3 +--
hw/block/virtio-blk.c | 16 +---
2 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h
index 5117431d96..f7345b0511 100644
--- a/include/hw/virtio/virtio-blk.h
+++ b/include/hw/virtio/virtio-blk.h
@@ -35,8 +35,6 @@ struct VirtIOBlkConf
BlockConf conf;
IOThread *iothread;
char *serial;
-uint32_t scsi;
-uint32_t config_wce;
uint32_t request_merging;
uint16_t num_queues;
uint16_t queue_size;
@@ -57,6 +55,7 @@ typedef struct VirtIOBlock {
bool dataplane_disabled;
bool dataplane_started;
struct VirtIOBlockDataPlane *dataplane;
+uint64_t host_features;
} VirtIOBlock;
typedef struct VirtIOBlockReq {
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index b2bb129efa..938273c63c 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -242,7 +242,7 @@ static int virtio_blk_handle_scsi_req(VirtIOBlockReq *req)
*/
scsi = (void *)elem->in_sg[elem->in_num - 2].iov_base;
-if (!blk->conf.scsi) {
+if (!virtio_has_feature(blk->host_features, VIRTIO_BLK_F_SCSI)) {
status = VIRTIO_BLK_S_UNSUPP;
goto fail;
}
@@ -783,12 +783,15 @@ static uint64_t virtio_blk_get_features(VirtIODevice
*vdev, uint64_t features,
{
VirtIOBlock *s = VIRTIO_BLK(vdev);
+/* Firstly sync all virtio-blk possible supported features */
+features |= s->host_features;
+
virtio_add_feature(, VIRTIO_BLK_F_SEG_MAX);
virtio_add_feature(, VIRTIO_BLK_F_GEOMETRY);
virtio_add_feature(, VIRTIO_BLK_F_TOPOLOGY);
virtio_add_feature(, VIRTIO_BLK_F_BLK_SIZE);
if (virtio_has_feature(features, VIRTIO_F_VERSION_1)) {
-if (s->conf.scsi) {
+if (virtio_has_feature(s->host_features, VIRTIO_BLK_F_SCSI)) {
error_setg(errp, "Please set scsi=off for virtio-blk devices in
order to use virtio 1.0");
return 0;
}
@@ -797,9 +800,6 @@ static uint64_t virtio_blk_get_features(VirtIODevice *vdev,
uint64_t features,
virtio_add_feature(, VIRTIO_BLK_F_SCSI);
}
-if (s->conf.config_wce) {
-virtio_add_feature(, VIRTIO_BLK_F_CONFIG_WCE);
-}
if (blk_enable_write_cache(s->blk)) {
virtio_add_feature(, VIRTIO_BLK_F_WCE);
}
@@ -1014,9 +1014,11 @@ static Property virtio_blk_properties[] = {
DEFINE_BLOCK_ERROR_PROPERTIES(VirtIOBlock, conf.conf),
DEFINE_BLOCK_CHS_PROPERTIES(VirtIOBlock, conf.conf),
DEFINE_PROP_STRING("serial", VirtIOBlock, conf.serial),
-DEFINE_PROP_BIT("config-wce", VirtIOBlock, conf.config_wce, 0, true),
+DEFINE_PROP_BIT64("config-wce", VirtIOBlock, host_features,
+ VIRTIO_BLK_F_CONFIG_WCE, true),
#ifdef __linux__
-DEFINE_PROP_BIT("scsi", VirtIOBlock, conf.scsi, 0, false),
+DEFINE_PROP_BIT64("scsi", VirtIOBlock, host_features,
+ VIRTIO_BLK_F_SCSI, false),
#endif
DEFINE_PROP_BIT("request-merging", VirtIOBlock, conf.request_merging, 0,
true),
--
2.20.1
Re: [Qemu-devel] [PATCH 1/4] pckbd: Convert DPRINTF->trace
On 2/12/19 2:47 PM, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert"
>
> Signed-off-by: Dr. David Alan Gilbert
Per https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg03186.html
this one already has:
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Stefan Hajnoczi
> ---
> hw/input/pckbd.c | 19 ++-
> hw/input/trace-events | 7 +++
> 2 files changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/hw/input/pckbd.c b/hw/input/pckbd.c
> index 72e7d5f6cc..47a606f5e3 100644
> --- a/hw/input/pckbd.c
> +++ b/hw/input/pckbd.c
> @@ -30,14 +30,7 @@
> #include "hw/input/i8042.h"
> #include "sysemu/sysemu.h"
>
> -/* debug PC keyboard */
> -//#define DEBUG_KBD
> -#ifdef DEBUG_KBD
> -#define DPRINTF(fmt, ...) \
> -do { printf("KBD: " fmt , ## __VA_ARGS__); } while (0)
> -#else
> -#define DPRINTF(fmt, ...)
> -#endif
> +#include "trace.h"
>
> /* Keyboard Controller Commands */
> #define KBD_CCMD_READ_MODE 0x20/* Read mode bits */
> @@ -210,7 +203,7 @@ static uint64_t kbd_read_status(void *opaque, hwaddr addr,
> KBDState *s = opaque;
> int val;
> val = s->status;
> -DPRINTF("kbd: read status=0x%02x\n", val);
> +trace_pckbd_kbd_read_status(val);
> return val;
> }
>
> @@ -224,7 +217,7 @@ static void kbd_queue(KBDState *s, int b, int aux)
>
> static void outport_write(KBDState *s, uint32_t val)
> {
> -DPRINTF("kbd: write outport=0x%02x\n", val);
> +trace_pckbd_outport_write(val);
> s->outport = val;
> qemu_set_irq(s->a20_out, (val >> 1) & 1);
> if (!(val & 1)) {
> @@ -237,7 +230,7 @@ static void kbd_write_command(void *opaque, hwaddr addr,
> {
> KBDState *s = opaque;
>
> -DPRINTF("kbd: write cmd=0x%02" PRIx64 "\n", val);
> +trace_pckbd_kbd_write_command(val);
>
> /* Bits 3-0 of the output port P2 of the keyboard controller may be
> pulsed
> * low for approximately 6 micro seconds. Bits 3-0 of the KBD_CCMD_PULSE
> @@ -326,7 +319,7 @@ static uint64_t kbd_read_data(void *opaque, hwaddr addr,
> else
> val = ps2_read_data(s->kbd);
>
> -DPRINTF("kbd: read data=0x%02x\n", val);
> +trace_pckbd_kbd_read_data(val);
> return val;
> }
>
> @@ -335,7 +328,7 @@ static void kbd_write_data(void *opaque, hwaddr addr,
> {
> KBDState *s = opaque;
>
> -DPRINTF("kbd: write data=0x%02" PRIx64 "\n", val);
> +trace_pckbd_kbd_write_data(val);
>
> switch(s->write_cmd) {
> case 0:
> diff --git a/hw/input/trace-events b/hw/input/trace-events
> index 3965a842ae..8e53ae5bbf 100644
> --- a/hw/input/trace-events
> +++ b/hw/input/trace-events
> @@ -14,6 +14,13 @@ adb_mouse_readreg(int reg, uint8_t val0, uint8_t val1)
> "reg %d obuf[0] 0x%2.2x o
> adb_mouse_request_change_addr(int devaddr) "change addr to 0x%x"
> adb_mouse_request_change_addr_and_handler(int devaddr, int handler) "change
> addr and handler to 0x%x, 0x%x"
>
> +# hw/input/pckbd.c
> +pckbd_kbd_read_data(uint32_t val) "0x%02x"
> +pckbd_kbd_read_status(int status) "0x%02x"
> +pckbd_outport_write(uint32_t val) "0x%02x"
> +pckbd_kbd_write_command(uint64_t val) "0x%02"PRIx64
> +pckbd_kbd_write_data(uint64_t val) "0x%02"PRIx64
> +
> # hw/input/ps2.c
> ps2_put_keycode(void *opaque, int keycode) "%p keycode 0x%02x"
> ps2_keyboard_event(void *opaque, int qcode, int down, unsigned int modifier,
> unsigned int modifiers) "%p qcode %d down %d modifier 0x%x modifiers 0x%x"
>
Re: [Qemu-devel] [PATCH] target/i386: Generate #UD when applying LOCK to a register
On 07/12/18 18:09, Richard Henderson wrote:
> This covers inc, dec, and the bit test instructions.
>
> I believe we've finally covered all of the cases for
> which we have an atomic path that would use the cpu_A0
> temp, which is only initialized for address sources.
>
> Fixes: https://bugs.launchpad.net/qemu/+bug/1803160/comments/4
> Signed-off-by: Richard Henderson
> ---
> target/i386/translate.c | 11 +++
> 1 file changed, 11 insertions(+)
>
> diff --git a/target/i386/translate.c b/target/i386/translate.c
> index 0dd5fbe45c..eb52322a47 100644
> --- a/target/i386/translate.c
> +++ b/target/i386/translate.c
> @@ -1398,6 +1398,11 @@ static void gen_op(DisasContext *s1, int op, TCGMemOp
> ot, int d)
> static void gen_inc(DisasContext *s1, TCGMemOp ot, int d, int c)
> {
> if (s1->prefix & PREFIX_LOCK) {
> +if (d != OR_TMP0) {
> +/* Lock prefix when destination is not memory. */
> +gen_illegal_opcode(s1);
> +return;
> +}
> tcg_gen_movi_tl(s1->T0, c > 0 ? 1 : -1);
> tcg_gen_atomic_add_fetch_tl(s1->T0, s1->A0, s1->T0,
> s1->mem_index, ot | MO_LE);
> @@ -6764,6 +6769,9 @@ static target_ulong disas_insn(DisasContext *s,
> CPUState *cpu)
> gen_op_ld_v(s, ot, s->T0, s->A0);
> }
> } else {
> +if (s->prefix & PREFIX_LOCK) {
> +goto illegal_op;
> +}
> gen_op_mov_v_reg(s, ot, s->T0, rm);
> }
> /* load shift */
> @@ -6803,6 +6811,9 @@ static target_ulong disas_insn(DisasContext *s,
> CPUState *cpu)
> gen_op_ld_v(s, ot, s->T0, s->A0);
> }
> } else {
> +if (s->prefix & PREFIX_LOCK) {
> +goto illegal_op;
> +}
> gen_op_mov_v_reg(s, ot, s->T0, rm);
> }
> bt_op:
>
Queued, thanks.
Paolo
Re: [Qemu-devel] [PATCH v4 0/6] virtio-blk: add DISCARD and WRITE_ZEROES features
On Fri, Feb 08, 2019 at 02:49:44PM +0100, Stefano Garzarella wrote: > This series adds the support of DISCARD and WRITE_ZEROES commands > and extends the virtio-blk-test to test WRITE_ZEROES command when > the feature is enabled. Looking at how this wasn't merged yet, maybe it's not too late. Series: Reviewed-by: Michael S. Tsirkin > v4: > - fixed error with mingw compiler in patch 4 > gcc and clang want %lu, but mingw wants %llu for BDRV_REQUEST_MAX_SECTORS. > Since is less than INT_MAX, I casted it to integer and I used %d in the > format string of error_setg. (mingw now is happy) > > v3: > - rebased on master (I removed Based-on tag since the new virtio headers from > linux v5.0-rc1 are merged) > - added patch 2 to add host_features field (as in virtio-net) [Michael] > - fixed patch 3 (previously 2/5) using the new host_features field > - fixed patch 4 (previously 3/5) following the Stefan's comments: > - fixed name of functions and fields > - used vdev and s pointers > - removed "wz-may-unmap" property > - split "dwz-max-sectors" in two properties > > v2: > - added patch 1 to use virtio_blk_handle_rw_error() with discard operation > - added patch 2 to make those new features machine-type dependent (thanks > David) > - fixed patch 3 (previously patch 1/2) adding more checks, block_acct_start() > for WRITE_ZEROES requests, and configurable parameters to > initialize the limits (max_sectors, wzeroes_may_unmap). > (thanks Stefan) > I moved in a new function the code to handle a single segment, > in order to simplify the support of multiple segments in the > future. > - added patch 4 to change the assert on data_size following the discussion > with > Thomas, Changpeng, Michael, and Stefan (thanks all) > - fixed patch 5 (previously patch 2/2) using local dwz_hdr variable instead of > dynamic allocation (thanks Thomas) > > Thanks, > Stefano > > Stefano Garzarella (6): > virtio-blk: add acct_failed param to virtio_blk_handle_rw_error() > virtio-blk: add host_features field in VirtIOBlock > virtio-blk: add "discard" and "write-zeroes" properties > virtio-blk: add DISCARD and WRITE_ZEROES features > tests/virtio-blk: change assert on data_size in virtio_blk_request() > tests/virtio-blk: add test for WRITE_ZEROES command > > hw/block/virtio-blk.c | 214 +++-- > hw/core/machine.c | 2 + > include/hw/virtio/virtio-blk.h | 5 +- > tests/virtio-blk-test.c| 75 +++- > 4 files changed, 282 insertions(+), 14 deletions(-) > > -- > 2.20.1
Re: [Qemu-devel] [PATCH 02/13] block: Freeze the backing chain for the duration of the commit job
Am 17.01.2019 um 16:33 hat Alberto Garcia geschrieben:
> Signed-off-by: Alberto Garcia
> ---
> block/commit.c | 8
> 1 file changed, 8 insertions(+)
>
> diff --git a/block/commit.c b/block/commit.c
> index 53148e610b..8824d135e0 100644
> --- a/block/commit.c
> +++ b/block/commit.c
> @@ -73,6 +73,8 @@ static int commit_prepare(Job *job)
> {
> CommitBlockJob *s = container_of(job, CommitBlockJob, common.job);
>
> +bdrv_unfreeze_backing_chain(s->commit_top_bs, s->base_bs);
> +
> /* Remove base node parent that still uses BLK_PERM_WRITE/RESIZE before
> * the normal backing chain can be restored. */
> blk_unref(s->base);
> @@ -89,6 +91,8 @@ static void commit_abort(Job *job)
> CommitBlockJob *s = container_of(job, CommitBlockJob, common.job);
> BlockDriverState *top_bs = blk_bs(s->top);
>
> +bdrv_unfreeze_backing_chain(s->commit_top_bs, s->base_bs);
commit_abort() will run if commit_prepare() returned an error, so we may
end up unfreezing twice. Maybe move it into the 'if (s->base)' block a
few lines down.
> /* Make sure commit_top_bs and top stay around until bdrv_replace_node()
> */
> bdrv_ref(top_bs);
> bdrv_ref(s->commit_top_bs);
> @@ -336,6 +340,10 @@ void commit_start(const char *job_id, BlockDriverState
> *bs,
> }
> }
>
> +if (bdrv_freeze_backing_chain(commit_top_bs, base, errp) < 0) {
> +goto fail;
> +}
Don't error paths need to unfreeze after this?
Kevin
Re: [Qemu-devel] [PATCH v2] virtio-blk: set correct config size for the host driver
> -Original Message-
> From: Michael S. Tsirkin [mailto:m...@redhat.com]
> Sent: Tuesday, February 12, 2019 9:49 PM
> To: Liu, Changpeng
> Cc: qemu-devel@nongnu.org; stefa...@redhat.com; sgarz...@redhat.com;
> dgilb...@redhat.com; ldok...@redhat.com
> Subject: Re: [PATCH v2] virtio-blk: set correct config size for the host
> driver
>
> On Tue, Feb 12, 2019 at 02:01:44PM +0800, Changpeng Liu wrote:
> > Commit caa1ee43 "vhost-user-blk: add discard/write zeroes features support"
> > introduced extra fields to existing struct virtio_blk_config, when
> > migration was executed from older QEMU version to current head, it
> > will break the migration. While here, set the correct config size
> > when initializing the host driver, for now, discard/write zeroes
> > are not supported by virtio-blk host driver, so set the config
> > size as before, users can change config size when adding the new
> > feature bits support.
> >
> > Signed-off-by: Changpeng Liu
>
>
> Pls rewrite commit log as suggested in this thread
> and repost.
Thanks, repost a v3 patch to address the commit message and your following
comments.
>
> > ---
> > hw/block/virtio-blk.c | 10 ++
> > 1 file changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
> > index 9a87b3b..846b7b9 100644
> > --- a/hw/block/virtio-blk.c
> > +++ b/hw/block/virtio-blk.c
> > @@ -28,6 +28,9 @@
> > #include "hw/virtio/virtio-bus.h"
> > #include "hw/virtio/virtio-access.h"
> >
> > +#define VIRTIO_BLK_CFG_SIZE (offsetof(struct virtio_blk_config, num_queues)
> + \
> > + sizeof_field(struct virtio_blk_config,
> > num_queues))
> > +
>
> I would just do offsetof(max_discard_sectors)
> with a comment "we don't support discard yet, hide associated config
> fields".
>
> > static void virtio_blk_init_request(VirtIOBlock *s, VirtQueue *vq,
> > VirtIOBlockReq *req)
> > {
> > @@ -761,7 +764,7 @@ static void virtio_blk_update_config(VirtIODevice *vdev,
> uint8_t *config)
> > blkcfg.alignment_offset = 0;
> > blkcfg.wce = blk_enable_write_cache(s->blk);
> > virtio_stw_p(vdev, _queues, s->conf.num_queues);
> > -memcpy(config, , sizeof(struct virtio_blk_config));
> > +memcpy(config, , VIRTIO_BLK_CFG_SIZE);
>
>
> Let's add QEMU_BUILD_BUG_ON(VIRTIO_BLK_CFG_SIZE <= sizeof(struct
> virtio_blk_config)) just for documentation purposes.
>
> > }
> >
> > static void virtio_blk_set_config(VirtIODevice *vdev, const uint8_t
> > *config)
> > @@ -769,7 +772,7 @@ static void virtio_blk_set_config(VirtIODevice *vdev,
> const uint8_t *config)
> > VirtIOBlock *s = VIRTIO_BLK(vdev);
> > struct virtio_blk_config blkcfg;
> >
> > -memcpy(, config, sizeof(blkcfg));
> > +memcpy(, config, VIRTIO_BLK_CFG_SIZE);
>
>
> Here too, QEMU_BUILD_BUG_ON(VIRTIO_BLK_CFG_SIZE <= sizeof(blkcfg))
>
> >
> > aio_context_acquire(blk_get_aio_context(s->blk));
> > blk_set_enable_write_cache(s->blk, blkcfg.wce != 0);
> > @@ -952,8 +955,7 @@ static void virtio_blk_device_realize(DeviceState *dev,
> Error **errp)
> > return;
> > }
> >
> > -virtio_init(vdev, "virtio-blk", VIRTIO_ID_BLOCK,
> > -sizeof(struct virtio_blk_config));
> > +virtio_init(vdev, "virtio-blk", VIRTIO_ID_BLOCK, VIRTIO_BLK_CFG_SIZE);
> >
> > s->blk = conf->conf.blk;
> > s->rq = NULL;
> > --
> > 1.9.3
Re: [Qemu-devel] [Qemu-block] [PATCH 06/13] block: Handle child references in bdrv_reopen_queue()
Am 12.02.2019 um 17:28 hat Kevin Wolf geschrieben:
> > -child_key_dot = g_strdup_printf("%s.", child->name);
> > -qdict_extract_subqdict(explicit_options, NULL, child_key_dot);
> > -qdict_extract_subqdict(options, _child_options, child_key_dot);
> > -g_free(child_key_dot);
> > +/* Check if the options contain a child reference */
> > +if (qdict_haskey(options, child->name)) {
> > +const char *childref = qdict_get_try_str(options, child->name);
> > +/*
> > + * The current child must not be reopened if the child
> > + * reference does not point to it.
> > + */
> > +if (g_strcmp0(childref, child->bs->node_name)) {
>
> This is where we would break the inheritance relationship.
Oops, correcting myself: It's not. We may only do that in the commit
stage, of course.
Kevin
Re: [Qemu-devel] [PULL 1/7] audio: fix pc speaker init
On 12.02.19 13:08, Philippe Mathieu-Daudé wrote:
> Hi David,
>
> On 2/12/19 12:47 PM, David Hildenbrand wrote:
>> On 24.01.19 14:20, Gerd Hoffmann wrote:
>>> Get rid of the pcspk_state global, allow pc speaker
>>> be added using "-device isa-pcspk".
>>>
>>> Signed-off-by: Gerd Hoffmann
>>> Reviewed-by: Philippe Mathieu-Daudé
>>> Message-id: 20190124110810.1040-1-kra...@redhat.com
>>> ---
>>> hw/audio/pcspk.c | 35 +++
>>> 1 file changed, 15 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/hw/audio/pcspk.c b/hw/audio/pcspk.c
>>> index 908696d483..b80a62ce90 100644
>>> --- a/hw/audio/pcspk.c
>>> +++ b/hw/audio/pcspk.c
>>> @@ -57,7 +57,6 @@ typedef struct {
>>> } PCSpkState;
>>>
>>> static const char *s_spk = "pcspk";
>>> -static PCSpkState *pcspk_state;
>>>
>>> static inline void generate_samples(PCSpkState *s)
>>> {
>>> @@ -111,22 +110,6 @@ static void pcspk_callback(void *opaque, int free)
>>> }
>>> }
>>>
>>> -static int pcspk_audio_init(ISABus *bus)
>>> -{
>>> -PCSpkState *s = pcspk_state;
>>> -struct audsettings as = {PCSPK_SAMPLE_RATE, 1, AUD_FMT_U8, 0};
>>> -
>>> -AUD_register_card(s_spk, >card);
>>> -
>>> -s->voice = AUD_open_out(>card, s->voice, s_spk, s, pcspk_callback,
>>> );
>>> -if (!s->voice) {
>>> -AUD_log(s_spk, "Could not open voice\n");
>>> -return -1;
>>> -}
>>> -
>>> -return 0;
>>> -}
>>> -
>>> static uint64_t pcspk_io_read(void *opaque, hwaddr addr,
>>>unsigned size)
>>> {
>>> @@ -179,12 +162,20 @@ static void pcspk_initfn(Object *obj)
>>>
>>> static void pcspk_realizefn(DeviceState *dev, Error **errp)
>>> {
>>> +struct audsettings as = {PCSPK_SAMPLE_RATE, 1, AUD_FMT_U8, 0};
>>> ISADevice *isadev = ISA_DEVICE(dev);
>>> PCSpkState *s = PC_SPEAKER(dev);
>>>
>>> isa_register_ioport(isadev, >ioport, s->iobase);
>>>
>>> -pcspk_state = s;
>>> +AUD_register_card(s_spk, >card);
>>> +
>>> +s->voice = AUD_open_out(>card, s->voice, s_spk, s, pcspk_callback,
>>> );
>>> +if (!s->voice) {
>>> +error_setg(errp, "Initializing audio voice failed");
>>> +AUD_remove_card(>card);
>>> +return;
>>> +}
>>> }
>>>
>>> static bool migrate_needed(void *opaque)
>>> @@ -221,8 +212,6 @@ static void pcspk_class_initfn(ObjectClass *klass, void
>>> *data)
>>> set_bit(DEVICE_CATEGORY_SOUND, dc->categories);
>>> dc->vmsd = _spk;
>>> dc->props = pcspk_properties;
>>> -/* Reason: realize sets global pcspk_state */
>>> -dc->user_creatable = false;
>>> }
>>>
>>> static const TypeInfo pcspk_info = {
>>> @@ -233,6 +222,12 @@ static const TypeInfo pcspk_info = {
>>> .class_init = pcspk_class_initfn,
>>> };
>>>
>>> +static int pcspk_audio_init(ISABus *bus)
>>> +{
>>> +isa_create_simple(bus, TYPE_PC_SPEAKER);
>>> +return 0;
>>> +}
>>> +
>>> static void pcspk_register(void)
>>> {
>>> type_register_static(_info);
>>>
>>
>> I suddenly get (under fedora 28)
>>
>> ALSA lib pulse.c:243:(pulse_connect) PulseAudio: Unable to connect:
>> Connection refused
>>
>> alsa: Could not initialize DAC
>> alsa: Failed to open `default':
>> alsa: Reason: Connection refused
>> ALSA lib pulse.c:243:(pulse_connect) PulseAudio: Unable to connect:
>> Connection refused
>>
>> alsa: Could not initialize DAC
>> alsa: Failed to open `default':
>> alsa: Reason: Connection refused
>
> This ALSA problem seems on your side.
>
>> audio: Failed to create voice `pcspk'
>> qemu-system-x86_64: Initialization of device isa-pcspk failed:
>> Initializing audio voice failed
>
> Previously the errors would be ignored and QEMU would start.
I just did a fedora 28 uupdate + reboot. Problem still exists. Would be
strange if only I would be hitting this problem with stock alsa libraries.
>
>>
>>
>> With
>>
>> sudo x86_64-softmmu/qemu-system-x86_64 \
>> --enable-kvm \
>> -m 4G,maxmem=40G,slots=2 \
>> -smp sockets=2,cores=2 \
>> -numa node,nodeid=0,cpus=0-1 -numa node,nodeid=1,cpus=2-3 \
>> -kernel /boot/vmlinuz-4.19.6-200.fc28.x86_64 \
>> -append "console=ttyS0 rd.shell rd.luks=0 rd.lvm=0 rd.md=0 rd.dm=0" \
>> -initrd /boot/initramfs-4.19.6-200.fc28.x86_64.img \
>> -machine pc,nvdimm \
>> -nographic \
>> -nodefaults \
>> -chardev stdio,id=serial \
>> --trace events=to_trace \
>> -device isa-serial,chardev=serial \
>> -chardev socket,id=monitor,path=/var/tmp/monitor,server,nowait \
>> -mon chardev=monitor,mode=readline
>>
>>
>> Could this be related to this patch? (or is maybe something about my
>> system messed up? will try rebooting, but other audio - e.g. via firefox
>> - works fine)
>
> Does your Firefox uses ALSA? The default install uses PulseAudio.
Yes, I think so. I will try to find another tool to test ALSA.
>
> I think the behavior change you are experiencing comes from the patch 7
> of this series "audio: probe audio drivers by default":
>
Re: [Qemu-devel] [PATCH v2 3/3] tpm_tis: fix format string specifier in tpm_tis_show_buffer()
On 2/11/19 10:13 PM, Stefan Berger wrote:
> On 2/11/19 3:09 PM, Liam Merwick wrote:
>> On 11/02/2019 19:56, Stefan Berger wrote:
>>> On 2/11/19 11:02 AM, Philippe Mathieu-Daudé wrote:
On 2/11/19 4:03 PM, Liam Merwick wrote:
[...]
> -printf("tpm_tis: %s length = %d\n", string, len);
> +printf("tpm_tis: %s length = %u\n", string, len);
So here the format is '%zu'.
However in code cleanup we try go get ride of printf() calls and
replace them with trace points.
>>>
>>>
>>> This code is only used for debugging if DEBUG_TIS has been #defined.
>>> No need to add tracing here.
>>
>> I'd come up the attached change (but that seems like overkill).
>
>
> I don't think we need tracing for this.
So if you think the code is mature enough, let's remove the DEBUG calls!
Else we prefer to convert DEBUG printf to trace events because (at least):
- no need to recompile to enable debugging
- when compiled with debugging, you don't mess with STDIO which can be
used as a chardev backend.
Thanks,
Phil.
[Qemu-devel] [RFC PULL 4/6] virtio-blk: add DISCARD and WRITE_ZEROES features
From: Stefano Garzarella
This patch adds the support of DISCARD and WRITE_ZEROES commands,
that have been introduced in the virtio-blk protocol to have
better performance when using SSD backend.
We support only one segment per request since multiple segments
are not widely used and there are no userspace APIs that allow
applications to submit multiple segments in a single call.
Reviewed-by: Michael S. Tsirkin
Reviewed-by: Stefan Hajnoczi
Signed-off-by: Stefano Garzarella
Acked-by: Pankaj Gupta
Message-id: 20190208134950.187665-5-sgarz...@redhat.com
Signed-off-by: Stefan Hajnoczi
---
include/hw/virtio/virtio-blk.h | 2 +
hw/block/virtio-blk.c | 184 +
2 files changed, 186 insertions(+)
diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h
index f7345b0511..015b523fe0 100644
--- a/include/hw/virtio/virtio-blk.h
+++ b/include/hw/virtio/virtio-blk.h
@@ -38,6 +38,8 @@ struct VirtIOBlkConf
uint32_t request_merging;
uint16_t num_queues;
uint16_t queue_size;
+uint32_t max_discard_sectors;
+uint32_t max_write_zeroes_sectors;
};
struct VirtIOBlockDataPlane;
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 6f2e86264d..5d1b823b66 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -147,6 +147,30 @@ out:
aio_context_release(blk_get_aio_context(s->conf.conf.blk));
}
+static void virtio_blk_discard_write_zeroes_complete(void *opaque, int ret)
+{
+VirtIOBlockReq *req = opaque;
+VirtIOBlock *s = req->dev;
+bool is_write_zeroes = (virtio_ldl_p(VIRTIO_DEVICE(s), >out.type) &
+~VIRTIO_BLK_T_BARRIER) ==
VIRTIO_BLK_T_WRITE_ZEROES;
+
+aio_context_acquire(blk_get_aio_context(s->conf.conf.blk));
+if (ret) {
+if (virtio_blk_handle_rw_error(req, -ret, false, is_write_zeroes)) {
+goto out;
+}
+}
+
+virtio_blk_req_complete(req, VIRTIO_BLK_S_OK);
+if (is_write_zeroes) {
+block_acct_done(blk_get_stats(s->blk), >acct);
+}
+virtio_blk_free_request(req);
+
+out:
+aio_context_release(blk_get_aio_context(s->conf.conf.blk));
+}
+
#ifdef __linux__
typedef struct {
@@ -480,6 +504,84 @@ static bool virtio_blk_sect_range_ok(VirtIOBlock *dev,
return true;
}
+static uint8_t virtio_blk_handle_discard_write_zeroes(VirtIOBlockReq *req,
+struct virtio_blk_discard_write_zeroes *dwz_hdr, bool is_write_zeroes)
+{
+VirtIOBlock *s = req->dev;
+VirtIODevice *vdev = VIRTIO_DEVICE(s);
+uint64_t sector;
+uint32_t num_sectors, flags, max_sectors;
+uint8_t err_status;
+int bytes;
+
+sector = virtio_ldq_p(vdev, _hdr->sector);
+num_sectors = virtio_ldl_p(vdev, _hdr->num_sectors);
+flags = virtio_ldl_p(vdev, _hdr->flags);
+max_sectors = is_write_zeroes ? s->conf.max_write_zeroes_sectors :
+ s->conf.max_discard_sectors;
+
+/*
+ * max_sectors is at most BDRV_REQUEST_MAX_SECTORS, this check
+ * make us sure that "num_sectors << BDRV_SECTOR_BITS" can fit in
+ * the integer variable.
+ */
+if (unlikely(num_sectors > max_sectors)) {
+err_status = VIRTIO_BLK_S_IOERR;
+goto err;
+}
+
+bytes = num_sectors << BDRV_SECTOR_BITS;
+
+if (unlikely(!virtio_blk_sect_range_ok(s, sector, bytes))) {
+err_status = VIRTIO_BLK_S_IOERR;
+goto err;
+}
+
+/*
+ * The device MUST set the status byte to VIRTIO_BLK_S_UNSUPP for discard
+ * and write zeroes commands if any unknown flag is set.
+ */
+if (unlikely(flags & ~VIRTIO_BLK_WRITE_ZEROES_FLAG_UNMAP)) {
+err_status = VIRTIO_BLK_S_UNSUPP;
+goto err;
+}
+
+if (is_write_zeroes) { /* VIRTIO_BLK_T_WRITE_ZEROES */
+int blk_aio_flags = 0;
+
+if (flags & VIRTIO_BLK_WRITE_ZEROES_FLAG_UNMAP) {
+blk_aio_flags |= BDRV_REQ_MAY_UNMAP;
+}
+
+block_acct_start(blk_get_stats(s->blk), >acct, bytes,
+ BLOCK_ACCT_WRITE);
+
+blk_aio_pwrite_zeroes(s->blk, sector << BDRV_SECTOR_BITS,
+ bytes, blk_aio_flags,
+ virtio_blk_discard_write_zeroes_complete, req);
+} else { /* VIRTIO_BLK_T_DISCARD */
+/*
+ * The device MUST set the status byte to VIRTIO_BLK_S_UNSUPP for
+ * discard commands if the unmap flag is set.
+ */
+if (unlikely(flags & VIRTIO_BLK_WRITE_ZEROES_FLAG_UNMAP)) {
+err_status = VIRTIO_BLK_S_UNSUPP;
+goto err;
+}
+
+blk_aio_pdiscard(s->blk, sector << BDRV_SECTOR_BITS, bytes,
+ virtio_blk_discard_write_zeroes_complete, req);
+}
+
+return VIRTIO_BLK_S_OK;
+
+err:
+if (is_write_zeroes) {
+block_acct_invalid(blk_get_stats(s->blk), BLOCK_ACCT_WRITE);
+}
+return err_status;
+}
+
static int virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb)
{
Re: [Qemu-devel] [PATCH] chardev/wctablet: Fix a typo
On Tue, Feb 12, 2019 at 4:12 PM Philippe Mathieu-Daudé
wrote:
>
> The correct name is Wacom.
> Fix the typo present this the origin of this file (378af96155d).
>
> Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Marc-André Lureau
> ---
> chardev/wctablet.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/chardev/wctablet.c b/chardev/wctablet.c
> index 969d014574..35dbd29a33 100644
> --- a/chardev/wctablet.c
> +++ b/chardev/wctablet.c
> @@ -177,7 +177,7 @@ static void wctablet_input_sync(DeviceState *dev)
> }
>
> static QemuInputHandler wctablet_handler = {
> -.name = "QEMU Wacome Pen Tablet",
> +.name = "QEMU Wacom Pen Tablet",
> .mask = INPUT_EVENT_MASK_BTN | INPUT_EVENT_MASK_ABS,
> .event = wctablet_input_event,
> .sync = wctablet_input_sync,
> --
> 2.20.1
>
>
--
Marc-André Lureau
Re: [Qemu-devel] [PATCH] s390x/kvm: add tracepoint to ioeventfd interface
On Tue, 12 Feb 2019 16:52:09 +0100
Philippe Mathieu-Daudé wrote:
> Hi Cornelia,
>
> On 2/12/19 4:30 PM, Cornelia Huck wrote:
> > Trace when assigning/unassigning.
> >
> > Signed-off-by: Cornelia Huck
> > ---
> > target/s390x/kvm.c| 2 ++
> > target/s390x/trace-events | 1 +
> > 2 files changed, 3 insertions(+)
> >
> > diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> > index 15fdc168e1c5..7d61bd109092 100644
> > --- a/target/s390x/kvm.c
> > +++ b/target/s390x/kvm.c
> > @@ -1886,6 +1886,8 @@ int kvm_s390_assign_subch_ioeventfd(EventNotifier
> > *notifier, uint32_t sch,
> > .addr = sch,
> > .len = 8,
> > };
> > +trace_kvm_s390_assign_subch_ioeventfd(kick.fd, kick.addr, assign,
> > + kick.datamatch);
> > if (!kvm_check_extension(kvm_state, KVM_CAP_IOEVENTFD)) {
> > return -ENOSYS;
> > }
> > diff --git a/target/s390x/trace-events b/target/s390x/trace-events
> > index a84e316e4937..bdc22f42cdbb 100644
> > --- a/target/s390x/trace-events
> > +++ b/target/s390x/trace-events
> > @@ -14,6 +14,7 @@ ioinst_chsc_cmd(uint16_t cmd, uint16_t len) "IOINST: chsc
> > command 0x%04x, len 0x
> > kvm_enable_cmma(int rc) "CMMA: enabling with result code %d"
> > kvm_clear_cmma(int rc) "CMMA: clearing with result code %d"
> > kvm_failed_cpu_state_set(int cpu_index, uint8_t state, const char *msg)
> > "Warning: Unable to set cpu %d state %" PRIu8 " to KVM: %s"
> > +kvm_s390_assign_subch_ioeventfd(int fd, uint32_t addr, bool assign, int
> > datamatch) "fd: %d sch: @0x%x assign: %d vq: %d"
>
> I noticed all s390x related trace events don't specify 's390' in the
> event name, maybe you can simply strip it. If you agree, feel free to
> apply that change directly yourself, no need for v2 ;)
Yeah; not exactly sure why the other events do that, though... I can
strip it if others agree.
>
> Regardless the name used:
> Reviewed-by: Philippe Mathieu-Daudé
Thanks!
>
> >
> > # target/s390x/cpu.c
> > cpu_set_state(int cpu_index, uint8_t state) "setting cpu %d state to %"
> > PRIu8
> >
Re: [Qemu-devel] [PATCH 06/13] block: Handle child references in bdrv_reopen_queue()
Am 17.01.2019 um 16:33 hat Alberto Garcia geschrieben:
> Children in QMP are specified with BlockdevRef / BlockdevRefOrNull,
> which can contain a set of child options, a child reference, or
> NULL. In optional attributes like "backing" it can also be missing.
>
> Only the first case (set of child options) is being handled properly
> by bdrv_reopen_queue(). This patch deals with all the others.
>
> Here's how these cases should be handled when bdrv_reopen_queue() is
> deciding what to do with each child of a BlockDriverState:
>
>1) Set of child options: the options are removed from the parent's
> options QDict and are passed to the child with a recursive
> bdrv_reopen_queue() call. This case was already working fine.
Small addition: This is only allowed if the child was implicitly created
(i.e. it inherits from the parent we're coming from).
>2) Child reference: there's two possibilites here.
>
> 2a) Reference to the current child: the child is put in the
> reopen queue, keeping its current set of options (since this
> was a child reference there was no way to specify a
> different set of options).
Why even put it in the reopen queue when nothing is going to change?
Ah, it's because inherited options might change, right?
But if the child did not inherit from this parent, we don't need to put
it into the queue anyway. The operation should still be allowed.
> 2b) Reference to a different BDS: the current child is not put
> in the reopen queue at all. Passing a reference to a
> different BDS can be used to replace a child, although at
> the moment no driver implements this, so it results in an
> error. In any case, the current child is not going to be
> reopened (and might in fact disappear if it's replaced)
Do we need to break a possible inheritance relationship between the
parent and the old child node in this case?
>3) NULL: This is similar to (2b). Although no driver allows this
> yet it can be used to detach the current child so it should not
> be put in the reopen queue.
Same comment as for 2b.
>4) Missing option: at the moment "backing" is the only case where
> this can happen. With "blockdev-add", leaving "backing" out
> means that the default backing file is opened. We don't want to
> open a new image during reopen, so we require that "backing" is
> always present. We'll relax this requirement a bit in the next
> patch.
I think this is only for keep_old_opts == false; otherwise it should be
treated the same as 2a to avoid breaking compatibility.
> Signed-off-by: Alberto Garcia
> ---
> block.c | 51
> ---
> include/block/block.h | 1 +
> 2 files changed, 45 insertions(+), 7 deletions(-)
>
> diff --git a/block.c b/block.c
> index 056a620eb2..fd51f1cd35 100644
> --- a/block.c
> +++ b/block.c
> @@ -3032,9 +3032,21 @@ static BlockReopenQueue
> *bdrv_reopen_queue_child(BlockReopenQueue *bs_queue,
> bs_entry->state.perm = UINT64_MAX;
> bs_entry->state.shared_perm = 0;
>
> +/*
> + * If keep_old_opts is false then it means that unspecified
> + * options must be reset to its original value. We don't allow
s/its/their/
> + * resetting 'backing' but we need to know if the option is
> + * missing in order to decide if we have to return an error.
> + */
> +if (!keep_old_opts) {
> +bs_entry->state.backing_missing =
> +!qdict_haskey(options, "backing") &&
> +!qdict_haskey(options, "backing.driver");
> +}
> +
> QLIST_FOREACH(child, >children, next) {
> -QDict *new_child_options;
> -char *child_key_dot;
> +QDict *new_child_options = NULL;
> +bool child_keep_old = keep_old_opts;
>
> /* reopen can only change the options of block devices that were
> * implicitly created and inherited options. For other (referenced)
> @@ -3043,13 +3055,31 @@ static BlockReopenQueue
> *bdrv_reopen_queue_child(BlockReopenQueue *bs_queue,
> continue;
> }
The 'continue' in the context just skipped any children that don't
inherit from this parent. Child options for those will stay unmodified
in the options dict.
For case 1, we'll later get an error because keys like 'child.foo' will
still be present and can't be processed by the driver. Implements
exactly what I commented above.
For case 2, the child isn't put in the reopen queue, and the child
reference can be used later. Matches my comment as well.
Good.
> -child_key_dot = g_strdup_printf("%s.", child->name);
> -qdict_extract_subqdict(explicit_options, NULL, child_key_dot);
> -qdict_extract_subqdict(options, _child_options, child_key_dot);
> -g_free(child_key_dot);
> +/* Check if the options contain a child reference */
> +if
[Qemu-devel] [PATCH] Discard old bitmap directories in QCOW2 image
Clean QCOW2 image from bitmap obsolete directory when a new one
is allocated and stored. It slows down the image growth a little bit.
The flag QCOW2_DISCARD_ALWAYS allows a call to raw_co_pdiscard()
that does the actual cleaning of the image on disk.
With the flag QCOW2_DISCARD_OTHER, a reference count of the cluster
is updated only.
Signed-off-by: Andrey Shinkevich
---
block/qcow2-bitmap.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
index b946301..682cb09 100644
--- a/block/qcow2-bitmap.c
+++ b/block/qcow2-bitmap.c
@@ -202,7 +202,7 @@ static void clear_bitmap_table(BlockDriverState *bs,
uint64_t *bitmap_table,
continue;
}
-qcow2_free_clusters(bs, addr, s->cluster_size, QCOW2_DISCARD_OTHER);
+qcow2_free_clusters(bs, addr, s->cluster_size, QCOW2_DISCARD_ALWAYS);
bitmap_table[i] = 0;
}
}
@@ -257,7 +257,7 @@ static int free_bitmap_clusters(BlockDriverState *bs,
Qcow2BitmapTable *tb)
clear_bitmap_table(bs, bitmap_table, tb->size);
qcow2_free_clusters(bs, tb->offset, tb->size * sizeof(uint64_t),
-QCOW2_DISCARD_OTHER);
+QCOW2_DISCARD_ALWAYS);
g_free(bitmap_table);
tb->offset = 0;
@@ -801,7 +801,7 @@ fail:
g_free(dir);
if (!in_place && dir_offset > 0) {
-qcow2_free_clusters(bs, dir_offset, dir_size, QCOW2_DISCARD_OTHER);
+qcow2_free_clusters(bs, dir_offset, dir_size, QCOW2_DISCARD_ALWAYS);
}
return ret;
@@ -904,14 +904,14 @@ static int update_ext_header_and_dir(BlockDriverState *bs,
}
if (old_size > 0) {
-qcow2_free_clusters(bs, old_offset, old_size, QCOW2_DISCARD_OTHER);
+qcow2_free_clusters(bs, old_offset, old_size, QCOW2_DISCARD_ALWAYS);
}
return 0;
fail:
if (new_offset > 0) {
-qcow2_free_clusters(bs, new_offset, new_size, QCOW2_DISCARD_OTHER);
+qcow2_free_clusters(bs, new_offset, new_size, QCOW2_DISCARD_ALWAYS);
}
s->bitmap_directory_offset = old_offset;
@@ -1242,7 +1242,7 @@ fail:
if (tb_offset > 0) {
qcow2_free_clusters(bs, tb_offset, tb_size * sizeof(tb[0]),
-QCOW2_DISCARD_OTHER);
+QCOW2_DISCARD_ALWAYS);
}
g_free(tb);
--
1.8.3.1
Re: [Qemu-devel] [qemu-s390x] [PATCH 09/15] s390-bios: ptr2u32 and u32toptr
On 2019-01-29 14:29, Jason J. Herne wrote:
> Introduce inline functions to convert between pointers and unsigned 32-bit
> ints. These are used to hide the ugliness required to avoid compiler
> warnings.
>
> Signed-off-by: Jason J. Herne
> ---
> pc-bios/s390-ccw/libc.h | 12
> 1 file changed, 12 insertions(+)
>
> diff --git a/pc-bios/s390-ccw/libc.h b/pc-bios/s390-ccw/libc.h
> index 818517f..e198f0b 100644
> --- a/pc-bios/s390-ccw/libc.h
> +++ b/pc-bios/s390-ccw/libc.h
> @@ -19,6 +19,18 @@ typedef unsigned short uint16_t;
> typedef unsigned int uint32_t;
> typedef unsigned long long uint64_t;
>
> +/* Avoids compiler warnings when casting a pointer to a u32 */
> +static inline uint32_t ptr2u32(void *ptr)
> +{
> +return (uint32_t)(uint64_t)ptr;
> +}
> +
> +/* Avoids compiler warnings when casting a u32 to a pointer */
> +static inline void *u32toptr(uint32_t n)
> +{
> +return (void *)(uint64_t)n;
> +}
Could you please put this into another header? libc.h is for
standard-compliant libc functions, and these are no standard functions,
as far as I know.
Thanks,
Thomas
Re: [Qemu-devel] [qemu-s390x] [PATCH 08/15] s390-bios: Map low core memory
On 2019-01-29 14:29, Jason J. Herne wrote:
> Create a new header for basic architecture specific definitions and add a
> mapping of low core memory. This mapping will be used by the real dasd boot
> process.
>
> Signed-off-by: Jason J. Herne
> ---
> pc-bios/s390-ccw/main.c | 2 +
> pc-bios/s390-ccw/s390-arch.h | 100
> +++
> 2 files changed, 102 insertions(+)
> create mode 100644 pc-bios/s390-ccw/s390-arch.h
>
> diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
> index 1bc61b5..fa90aa3 100644
> --- a/pc-bios/s390-ccw/main.c
> +++ b/pc-bios/s390-ccw/main.c
> @@ -9,6 +9,7 @@
> */
>
> #include "libc.h"
> +#include "s390-arch.h"
> #include "s390-ccw.h"
> #include "cio.h"
> #include "virtio.h"
> @@ -19,6 +20,7 @@ static char loadparm_str[LOADPARM_LEN + 1] = { 0, 0, 0, 0,
> 0, 0, 0, 0, 0 };
> QemuIplParameters qipl;
> IplParameterBlock iplb __attribute__((__aligned__(PAGE_SIZE)));
> static bool have_iplb;
> +const LowCore *lowcore; /* Yes, this *is* a pointer to address 0 */
>
> #define LOADPARM_PROMPT "PROMPT "
> #define LOADPARM_EMPTY ""
> diff --git a/pc-bios/s390-ccw/s390-arch.h b/pc-bios/s390-ccw/s390-arch.h
> new file mode 100644
> index 000..47eaa04
> --- /dev/null
> +++ b/pc-bios/s390-ccw/s390-arch.h
> @@ -0,0 +1,100 @@
> +/*
> + * S390 Basic Architecture
> + *
> + * Copyright (c) 2018 Jason J. Herne
2019 ?
> + * This work is licensed under the terms of the GNU GPL, version 2 or (at
> + * your option) any later version. See the COPYING file in the top-level
> + * directory.
> + */
> +
> +#ifndef S390_ARCH_H
> +#define S390_ARCH_H
> +
> +typedef struct PSW {
> +uint64_t mask;
> +uint64_t addr;
> +} __attribute__ ((packed, aligned(8))) PSW;
We've seen quite some trouble with "packed" in the past... See
3b8afb41bc8e or 55281a2c53b884d0 for example ... This is only the
s390-ccw bios code here, so it is likely ok, but still, since this
structure is "naturally" packed, I'd rather go without that attribute
here (even if it's only to allow the compiler to generate better code in
some cases). You could still _Static_assert(sizeof(struct PSW) == 16)
afterwards, just to be sure.
> +
> +/* Older PSW format used by LPSW instruction */
> +typedef struct PSWLegacy {
> +uint32_t mask;
> +uint32_t addr;
> +} __attribute__ ((packed, aligned(8))) PSWLegacy;
dito, I'd drop the packed attribute here.
> +/* s390 psw bit masks */
> +#define PSW_MASK_IOINT 0x0200ULL
> +#define PSW_MASK_WAIT 0x0002ULL
> +#define PSW_MASK_EAMODE 0x0001ULL
> +#define PSW_MASK_BAMODE 0x8000ULL
> +#define PSW_MASK_ZMODE (PSW_MASK_EAMODE | PSW_MASK_BAMODE)
> +
> +/* Low core mapping */
> +typedef struct LowCore {
> +/* prefix area: defined by architecture */
> +uint64_tipl_psw; /* 0x000 */
No PSWLegacy here? ;-)
> +uint32_tccw1[2]; /* 0x008 */
> +uint32_tccw2[2]; /* 0x010 */
> +uint8_t pad1[0x80 - 0x18];/* 0x018 */
> +uint32_text_params; /* 0x080 */
> +uint16_tcpu_addr; /* 0x084 */
> +uint16_text_int_code; /* 0x086 */
> +uint16_tsvc_ilen; /* 0x088 */
> +uint16_tsvc_code; /* 0x08a */
> +uint16_tpgm_ilen; /* 0x08c */
> +uint16_tpgm_code; /* 0x08e */
> +uint32_tdata_exc_code;/* 0x090 */
> +uint16_tmon_class_num;/* 0x094 */
> +uint16_tper_perc_atmid; /* 0x096 */
> +uint64_tper_address; /* 0x098 */
> +uint8_t exc_access_id;/* 0x0a0 */
> +uint8_t per_access_id;/* 0x0a1 */
> +uint8_t op_access_id; /* 0x0a2 */
> +uint8_t ar_access_id; /* 0x0a3 */
> +uint8_t pad2[0xA8 - 0xA4];/* 0x0a4 */
> +uint64_ttrans_exc_code; /* 0x0a8 */
> +uint64_tmonitor_code; /* 0x0b0 */
> +uint16_tsubchannel_id;/* 0x0b8 */
> +uint16_tsubchannel_nr;/* 0x0ba */
> +uint32_tio_int_parm; /* 0x0bc */
> +uint32_tio_int_word; /* 0x0c0 */
> +uint8_t pad3[0xc8 - 0xc4];/* 0x0c4 */
> +uint32_tstfl_fac_list;/* 0x0c8 */
> +uint8_t pad4[0xe8 - 0xcc];/* 0x0cc */
> +uint64_tmcic; /* 0x0e8 */
> +uint8_t pad5[0xf4 - 0xf0];/* 0x0f0 */
> +uint32_texternal_damage_code; /* 0x0f4 */
> +uint64_tfailing_storage_address; /* 0x0f8 */
> +uint8_t pad6[0x110 - 0x100]; /* 0x100 */
> +uint64_t
Re: [Qemu-devel] [PATCH v2 3/4] tests/migration-test: Add a test for ignore-shared capability
11.02.2019, 16:17, "Dr. David Alan Gilbert" :
> * Yury Kotov (yury-ko...@yandex-team.ru) wrote:
>> Signed-off-by: Yury Kotov
>> ---
>> tests/migration-test.c | 109 +
>> 1 file changed, 89 insertions(+), 20 deletions(-)
>>
>> diff --git a/tests/migration-test.c b/tests/migration-test.c
>> index 8352612364..485f42b2d2 100644
>> --- a/tests/migration-test.c
>> +++ b/tests/migration-test.c
>> @@ -332,6 +332,13 @@ static void cleanup(const char *filename)
>> g_free(path);
>> }
>>
>> +static char *get_shmem_opts(const char *mem_size, const char *shmem_path)
>> +{
>> + return g_strdup_printf("-object memory-backend-file,id=mem0,size=%s"
>> + ",mem-path=%s,share=on -numa node,memdev=mem0",
>> + mem_size, shmem_path);
>> +}
>> +
>> static void migrate_check_parameter(QTestState *who, const char *parameter,
>> long long value)
>> {
>> @@ -430,73 +437,91 @@ static void migrate_postcopy_start(QTestState *from,
>> QTestState *to)
>> }
>>
>> static int test_migrate_start(QTestState **from, QTestState **to,
>> - const char *uri, bool hide_stderr)
>> + const char *uri, bool hide_stderr,
>> + bool use_shmem)
>> {
>> gchar *cmd_src, *cmd_dst;
>> char *bootpath = g_strdup_printf("%s/bootsect", tmpfs);
>> + char *extra_opts = NULL;
>> + char *shmem_path = NULL;
>> const char *arch = qtest_get_arch();
>> const char *accel = "kvm:tcg";
>>
>> got_stop = false;
>>
>> + if (use_shmem) {
>> + shmem_path = g_strdup_printf("/dev/shm/qemu-%d", getpid());
>> + }
>
> I think /dev/shm is non-portable; so I think you'll need to have a way
> to skip the test on OSs that don't have it.
>
Ok, will fix in v3.
>> if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
>> init_bootfile(bootpath, x86_bootsect);
>> + extra_opts = use_shmem ? get_shmem_opts("150M", shmem_path) : NULL;
>> cmd_src = g_strdup_printf("-machine accel=%s -m 150M"
>> " -name source,debug-threads=on"
>> " -serial file:%s/src_serial"
>> - " -drive file=%s,format=raw",
>> - accel, tmpfs, bootpath);
>> + " -drive file=%s,format=raw %s",
>> + accel, tmpfs, bootpath,
>> + extra_opts ? extra_opts : "");
>
> It's painful to have to use the ?: here as well as above where you set
> extra_opts, but I guess you need to, to allow you to free extra_opts
> below.
>
I'll think how to write it more clear. May be it's better to g_strdup("")
instead of NULL to eliminate some of ?:.
>> cmd_dst = g_strdup_printf("-machine accel=%s -m 150M"
>> " -name target,debug-threads=on"
>> " -serial file:%s/dest_serial"
>> " -drive file=%s,format=raw"
>> - " -incoming %s",
>> - accel, tmpfs, bootpath, uri);
>> + " -incoming %s %s",
>> + accel, tmpfs, bootpath, uri,
>> + extra_opts ? extra_opts : "");
>> start_address = X86_TEST_MEM_START;
>> end_address = X86_TEST_MEM_END;
>> } else if (g_str_equal(arch, "s390x")) {
>> init_bootfile_s390x(bootpath);
>> + extra_opts = use_shmem ? get_shmem_opts("128M", shmem_path) : NULL;
>> cmd_src = g_strdup_printf("-machine accel=%s -m 128M"
>> " -name source,debug-threads=on"
>> - " -serial file:%s/src_serial -bios %s",
>> - accel, tmpfs, bootpath);
>> + " -serial file:%s/src_serial -bios %s %s",
>> + accel, tmpfs, bootpath,
>> + extra_opts ? extra_opts : "");
>> cmd_dst = g_strdup_printf("-machine accel=%s -m 128M"
>> " -name target,debug-threads=on"
>> " -serial file:%s/dest_serial -bios %s"
>> - " -incoming %s",
>> - accel, tmpfs, bootpath, uri);
>> + " -incoming %s %s",
>> + accel, tmpfs, bootpath, uri,
>> + extra_opts ? extra_opts : "");
>> start_address = S390_TEST_MEM_START;
>> end_address = S390_TEST_MEM_END;
>> } else if (strcmp(arch, "ppc64") == 0) {
>> + extra_opts = use_shmem ? get_shmem_opts("256M", shmem_path) : NULL;
>> cmd_src = g_strdup_printf("-machine accel=%s -m 256M -nodefaults"
>> " -name source,debug-threads=on"
>> " -serial file:%s/src_serial"
>> " -prom-env 'use-nvramrc?=true'
>> -prom-env "
>> "'nvramrc=hex .\" _\" begin %x %x "
>> "do i c@ 1 + i c! 1000 +loop .\" B\" 0 "
>> - "until'", accel, tmpfs, end_address,
>> - start_address);
>> + "until' %s", accel, tmpfs, end_address,
>> + start_address, extra_opts ? extra_opts : "");
>> cmd_dst = g_strdup_printf("-machine accel=%s -m 256M"
>> " -name target,debug-threads=on"
[Qemu-devel] [PATCH 2/4] HMP: Prepend errors with 'Error:'
From: "Dr. David Alan Gilbert"
Always make error messages start with 'Error:' as a fallback
to make sure that anything parsing them can tell it failed.
Note: Some places don't use hmp_handle_error
Signed-off-by: Dr. David Alan Gilbert
---
hmp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hmp.c b/hmp.c
index b2a2b1f84e..4d14718fea 100644
--- a/hmp.c
+++ b/hmp.c
@@ -62,7 +62,7 @@ static void hmp_handle_error(Monitor *mon, Error **errp)
{
assert(errp);
if (*errp) {
-error_report_err(*errp);
+error_reportf_err(*errp, "Error: ");
}
}
--
2.20.1
Re: [Qemu-devel] [PULL 00/25] pci, pc, virtio: fixes, cleanups, features
On Tue, Feb 12, 2019 at 02:53:16PM +0100, Philippe Mathieu-Daudé wrote: > On 2/12/19 2:24 PM, Michael S. Tsirkin wrote: > > On Tue, Feb 12, 2019 at 02:15:36PM +0100, Philippe Mathieu-Daudé wrote: > >> On 2/12/19 2:04 PM, Michael S. Tsirkin wrote: > >>> On Tue, Feb 12, 2019 at 11:39:21AM +0100, Philippe Mathieu-Daudé wrote: > On 2/12/19 8:11 AM, Peter Xu wrote: > > On Tue, Feb 05, 2019 at 11:06:42AM -0500, Michael S. Tsirkin wrote: > >> > >> OK I reverted the whole part dealing with vhost-user and reposted. > > > > I noticed that the merged pull request could possibly have squashed > > the below two patches (in previous pull) into one super patch > > (a56de056c91f87e1e): > > > > i386/kvm: ignore masked irqs when update msi routes > > contrib/vhost-user-blk: fix the compilation issue > > > > Here, the first patch lost its commit message, and the last patch lost > > its real author. ;) > > I suggest we revert a56de056c9 ASAP and reapply the both patches, this > will ease cherry-picking/downstream workflow. > >>> > >>> I don't see why does it help upstream. > >> > >> I'd have suggested the same if I had no idea what 'downstream workflow' > >> mean, simply to keep the tree clear and avoid to have unrelated changes > >> squashed altogether. > >> Commit a56de056c9 really looks messy. MSI/MSIX changes described by "fix > >> vhost-user-blk compilation". > >> Hopefully it won't trigger any problem which requires bisecting to it, > >> then contact Changpeng Liu asking him what he intented to do with his > >> commit. > >> Your call anyway :) > >> > >> Regards, > >> > >> Phil. > > > > > > OK these are good points. I'm not sure what happened but it looks like I > > screwed up when resolving some conflicts. Care posting a patchset > > looking sane? > > Yes, will do. Thanks! Include the reverts in it pls. -- MST
[Qemu-devel] [PATCH 0/3] pci, vhost-user: Fix two incorrectly applied patches
Commit a56de056c91f8 squashed two unrelated commits at once. Revert it and reapply the two commits to avoid confusion. See: https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg02966.html Changpeng Liu (1): contrib/vhost-user-blk: fix the compilation issue Peter Xu (1): i386/kvm: ignore masked irqs when update msi routes Philippe Mathieu-Daudé (1): Revert "contrib/vhost-user-blk: fix the compilation issue" -- 2.20.1
[Qemu-devel] [RFC PULL 6/6] tests/virtio-blk: add test for WRITE_ZEROES command
From: Stefano Garzarella
If the WRITE_ZEROES feature is enabled, we check this command
in the test_basic().
Reviewed-by: Michael S. Tsirkin
Reviewed-by: Stefan Hajnoczi
Acked-by: Thomas Huth
Signed-off-by: Stefano Garzarella
Acked-by: Pankaj Gupta
Message-id: 20190208134950.187665-7-sgarz...@redhat.com
Signed-off-by: Stefan Hajnoczi
---
tests/virtio-blk-test.c | 60 +
1 file changed, 60 insertions(+)
diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
index 0739498da7..35bd92dbfc 100644
--- a/tests/virtio-blk-test.c
+++ b/tests/virtio-blk-test.c
@@ -244,6 +244,66 @@ static void test_basic(QVirtioDevice *dev, QGuestAllocator
*alloc,
guest_free(alloc, req_addr);
+if (features & (1u << VIRTIO_BLK_F_WRITE_ZEROES)) {
+struct virtio_blk_discard_write_zeroes dwz_hdr;
+void *expected;
+
+/*
+ * WRITE_ZEROES request on the same sector of previous test where
+ * we wrote "TEST".
+ */
+req.type = VIRTIO_BLK_T_WRITE_ZEROES;
+req.data = (char *) _hdr;
+dwz_hdr.sector = 0;
+dwz_hdr.num_sectors = 1;
+dwz_hdr.flags = 0;
+
+req_addr = virtio_blk_request(alloc, dev, , sizeof(dwz_hdr));
+
+free_head = qvirtqueue_add(vq, req_addr, 16, false, true);
+qvirtqueue_add(vq, req_addr + 16, sizeof(dwz_hdr), false, true);
+qvirtqueue_add(vq, req_addr + 16 + sizeof(dwz_hdr), 1, true, false);
+
+qvirtqueue_kick(dev, vq, free_head);
+
+qvirtio_wait_used_elem(dev, vq, free_head, NULL,
+ QVIRTIO_BLK_TIMEOUT_US);
+status = readb(req_addr + 16 + sizeof(dwz_hdr));
+g_assert_cmpint(status, ==, 0);
+
+guest_free(alloc, req_addr);
+
+/* Read request to check if the sector contains all zeroes */
+req.type = VIRTIO_BLK_T_IN;
+req.ioprio = 1;
+req.sector = 0;
+req.data = g_malloc0(512);
+
+req_addr = virtio_blk_request(alloc, dev, , 512);
+
+g_free(req.data);
+
+free_head = qvirtqueue_add(vq, req_addr, 16, false, true);
+qvirtqueue_add(vq, req_addr + 16, 512, true, true);
+qvirtqueue_add(vq, req_addr + 528, 1, true, false);
+
+qvirtqueue_kick(dev, vq, free_head);
+
+qvirtio_wait_used_elem(dev, vq, free_head, NULL,
+ QVIRTIO_BLK_TIMEOUT_US);
+status = readb(req_addr + 528);
+g_assert_cmpint(status, ==, 0);
+
+data = g_malloc(512);
+expected = g_malloc0(512);
+memread(req_addr + 16, data, 512);
+g_assert_cmpmem(data, 512, expected, 512);
+g_free(expected);
+g_free(data);
+
+guest_free(alloc, req_addr);
+}
+
if (features & (1u << VIRTIO_F_ANY_LAYOUT)) {
/* Write and read with 2 descriptor layout */
/* Write request */
--
2.20.1
Re: [Qemu-devel] [PATCH 04/13] block: Freeze the backing chain for the duration of the stream job
Am 17.01.2019 um 16:33 hat Alberto Garcia geschrieben:
> Signed-off-by: Alberto Garcia
> ---
> block/stream.c | 16
> 1 file changed, 16 insertions(+)
>
> diff --git a/block/stream.c b/block/stream.c
> index 7a49ac0992..39a2e10892 100644
> --- a/block/stream.c
> +++ b/block/stream.c
> @@ -54,6 +54,14 @@ static int coroutine_fn stream_populate(BlockBackend *blk,
> return blk_co_preadv(blk, offset, qiov.size, ,
> BDRV_REQ_COPY_ON_READ);
> }
>
> +static void stream_abort(Job *job)
> +{
> +StreamBlockJob *s = container_of(job, StreamBlockJob, common.job);
> +BlockJob *bjob = >common;
> +
> +bdrv_unfreeze_backing_chain(blk_bs(bjob->blk), s->base);
> +}
Like in commit, you can get a double unfreeze here if .abort is called
after .prepare returned an error.
Kevin
Re: [Qemu-devel] [PATCH 04/13] block: Freeze the backing chain for the duration of the stream job
On Tue 12 Feb 2019 04:15:58 PM CET, Kevin Wolf wrote:
> Am 17.01.2019 um 16:33 hat Alberto Garcia geschrieben:
>> Signed-off-by: Alberto Garcia
>> ---
>> block/stream.c | 16
>> 1 file changed, 16 insertions(+)
>>
>> diff --git a/block/stream.c b/block/stream.c
>> index 7a49ac0992..39a2e10892 100644
>> --- a/block/stream.c
>> +++ b/block/stream.c
>> @@ -54,6 +54,14 @@ static int coroutine_fn stream_populate(BlockBackend *blk,
>> return blk_co_preadv(blk, offset, qiov.size, ,
>> BDRV_REQ_COPY_ON_READ);
>> }
>>
>> +static void stream_abort(Job *job)
>> +{
>> +StreamBlockJob *s = container_of(job, StreamBlockJob, common.job);
>> +BlockJob *bjob = >common;
>> +
>> +bdrv_unfreeze_backing_chain(blk_bs(bjob->blk), s->base);
>> +}
>
> Like in commit, you can get a double unfreeze here if .abort is called
> after .prepare returned an error.
You're right, I'll fix it
Berto
Re: [Qemu-devel] [PATCH] hw/dma/i8257: Use qemu_log_mask(UNIMP) instead of fprintf
On 2019-02-12 15:53, Philippe Mathieu-Daudé wrote:
> Avoid to clutter stdout until explicitly requested (with -d unimp):
>
> $ qemu-system-mips64el -M fulong2e -bios pmon_2e.bin
> dma: command df not supported
> dma: command df not supported
> dma: command df not supported
> dma: command df not supported
>
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> hw/dma/i8257.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/hw/dma/i8257.c b/hw/dma/i8257.c
> index 52675e97c9..3e1f13a4aa 100644
> --- a/hw/dma/i8257.c
> +++ b/hw/dma/i8257.c
> @@ -26,6 +26,7 @@
> #include "hw/isa/isa.h"
> #include "hw/dma/i8257.h"
> #include "qemu/main-loop.h"
> +#include "qemu/log.h"
> #include "trace.h"
>
> #define I8257(obj) \
> @@ -185,7 +186,8 @@ static void i8257_write_cont(void *opaque, hwaddr nport,
> uint64_t data,
> switch (iport) {
> case 0x00: /* command */
> if ((data != 0) && (data & CMD_NOT_SUPPORTED)) {
> -dolog("command %"PRIx64" not supported\n", data);
> +qemu_log_mask(LOG_UNIMP, "%s: cmd 0x%02"PRIx64" not supported\n",
> + __func__, data);
> return;
> }
> d->command = data;
>
Reviewed-by: Thomas Huth
[Qemu-devel] [PATCH 6/7] build-sys: link with slirp as an external project
Use the "system" libslirp if its present or requested.
Else build with a static libslirp.a if slirp/ is checked
out ("internal") or a submodule ("git").
Signed-off-by: Marc-André Lureau
---
net/slirp.c| 2 +-
Makefile | 8 +++---
Makefile.objs | 1 -
Makefile.target| 5 +---
configure | 65 +++---
net/Makefile.objs | 2 ++
util/Makefile.objs | 1 +
7 files changed, 72 insertions(+), 12 deletions(-)
diff --git a/net/slirp.c b/net/slirp.c
index b3bb1880c7..9c69500a36 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -37,7 +37,7 @@
#include "monitor/monitor.h"
#include "qemu/error-report.h"
#include "qemu/sockets.h"
-#include "slirp/src/libslirp.h"
+#include
#include "chardev/char-fe.h"
#include "sysemu/sysemu.h"
#include "qemu/cutils.h"
diff --git a/Makefile b/Makefile
index 3658310b95..55188d9e0a 100644
--- a/Makefile
+++ b/Makefile
@@ -382,8 +382,7 @@ dummy := $(call unnest-vars,, \
ui-obj-m \
audio-obj-y \
audio-obj-m \
-trace-obj-y \
-slirp-obj-y)
+trace-obj-y)
include $(SRC_PATH)/tests/Makefile.include
@@ -456,7 +455,10 @@ CAP_CFLAGS += -DCAPSTONE_HAS_X86
subdir-capstone: .git-submodule-status
$(call quiet-command,$(MAKE) -C $(SRC_PATH)/capstone CAPSTONE_SHARED=no
BUILDDIR="$(BUILD_DIR)/capstone" CC="$(CC)" AR="$(AR)" LD="$(LD)"
RANLIB="$(RANLIB)" CFLAGS="$(CAP_CFLAGS)" $(SUBDIR_MAKEFLAGS)
$(BUILD_DIR)/capstone/$(LIBCAPSTONE))
-$(SUBDIR_RULES): libqemuutil.a $(common-obj-y) $(chardev-obj-y) $(slirp-obj-y)
\
+subdir-slirp: .git-submodule-status
+ $(call quiet-command,$(MAKE) -C $(SRC_PATH)/slirp
BUILD_DIR="$(BUILD_DIR)/slirp" CC="$(CC)" AR="$(AR)" LD="$(LD)"
RANLIB="$(RANLIB)" CFLAGS="$(QEMU_CFLAGS)")
+
+$(SUBDIR_RULES): libqemuutil.a $(common-obj-y) $(chardev-obj-y) \
$(qom-obj-y) $(crypto-aes-obj-$(CONFIG_USER_ONLY))
ROMSUBDIR_RULES=$(patsubst %,romsubdir-%, $(ROMS))
diff --git a/Makefile.objs b/Makefile.objs
index b7aae33367..90a7edeb4b 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -17,7 +17,6 @@ util-obj-y += $(QAPI_MODULES:%=qapi/qapi-events-%.o)
util-obj-y += qapi/qapi-introspect.o
chardev-obj-y = chardev/
-slirp-obj-$(CONFIG_SLIRP) = slirp/
###
# block-obj-y is code used by both qemu system emulation and qemu-img
diff --git a/Makefile.target b/Makefile.target
index 401dc1ea6f..cb85eb450c 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -173,7 +173,6 @@ target-obj-y :=
block-obj-y :=
common-obj-y :=
chardev-obj-y :=
-slirp-obj-y :=
include $(SRC_PATH)/Makefile.objs
dummy := $(call unnest-vars,,target-obj-y)
target-obj-y-save := $(target-obj-y)
@@ -186,8 +185,7 @@ dummy := $(call unnest-vars,.., \
qom-obj-y \
io-obj-y \
common-obj-y \
- common-obj-m \
- slirp-obj-y)
+ common-obj-m)
target-obj-y := $(target-obj-y-save)
all-obj-y += $(common-obj-y)
all-obj-y += $(target-obj-y)
@@ -196,7 +194,6 @@ all-obj-$(CONFIG_SOFTMMU) += $(block-obj-y) $(chardev-obj-y)
all-obj-$(CONFIG_USER_ONLY) += $(crypto-aes-obj-y)
all-obj-$(CONFIG_SOFTMMU) += $(crypto-obj-y)
all-obj-$(CONFIG_SOFTMMU) += $(io-obj-y)
-all-obj-$(CONFIG_SOFTMMU) += $(slirp-obj-y)
$(QEMU_PROG_BUILD): config-devices.mak
diff --git a/configure b/configure
index fbd0825488..3664911a14 100755
--- a/configure
+++ b/configure
@@ -406,7 +406,7 @@ includedir="\${prefix}/include"
sysconfdir="\${prefix}/etc"
local_statedir="\${prefix}/var"
confsuffix="/qemu"
-slirp="yes"
+slirp=""
oss_lib=""
bsd="no"
linux="no"
@@ -1106,6 +1106,10 @@ for opt do
;;
--disable-slirp) slirp="no"
;;
+ --enable-slirp=git) slirp="git"
+ ;;
+ --enable-slirp=system) slirp="system"
+ ;;
--disable-vde) vde="no"
;;
--enable-vde) vde="yes"
@@ -5690,6 +5694,55 @@ if test "$libpmem" != "no"; then
fi
fi
+##
+# check for slirp
+
+case "$slirp" in
+ "" | yes)
+if $pkg_config slirp; then
+ slirp=system
+elif test -e "${source_path}/.git" && test $git_update = 'yes' ; then
+ slirp=git
+elif test -e "${source_path}/slirp/Makefile" ; then
+ slirp=internal
+elif test -z "$slirp" ; then
+ slirp=no
+else
+ feature_not_found "slirp" "Install slirp devel or git submodule"
+fi
+;;
+
+ system)
+if ! $pkg_config slirp; then
+ feature_not_found "slirp" "Install slirp devel"
+fi
+;;
+esac
+
+case "$slirp" in
+ git | internal)
+if test "$slirp" = git; then
+ git_submodules="${git_submodules} slirp"
+fi
+mkdir -p slirp
+slirp_cflags="-I\$(SRC_PATH)/slirp/src -I\$(BUILD_DIR)/slirp/src"
+slirp_libs="-L\$(BUILD_DIR)/slirp -lslirp"
+;;
+
+ system)
+slirp_version=$($pkg_config --modversion slirp 2>/dev/null)
+
[Qemu-devel] [PATCH 5/7] slirp: add a standalone Makefile
Add a simple Makefile to build libslirp.a, a static library version of libslirp, to be used by QEMU during a transition period, until a shared library is available. Signed-off-by: Marc-André Lureau --- slirp/Makefile | 47 +++ 1 file changed, 47 insertions(+) create mode 100644 slirp/Makefile diff --git a/slirp/Makefile b/slirp/Makefile new file mode 100644 index 00..6d48f626ba --- /dev/null +++ b/slirp/Makefile @@ -0,0 +1,47 @@ +ROOT_DIR := $(shell dirname $(realpath $(lastword $(MAKEFILE_LIST +BUILD_DIR ?= . + +LIBSLIRP = $(BUILD_DIR)/libslirp.a + +all: $(LIBSLIRP) + +SRCS := $(wildcard src/*.c) +OBJS := $(SRCS:%.c=$(BUILD_DIR)/%.o) +DEPS := $(OBJS:%.o=%.d) + +INC_DIRS := $(BUILD_DIR)/src +INC_FLAGS := $(addprefix -I,$(INC_DIRS)) + +override CFLAGS += \ + -DG_LOG_DOMAIN='"Slirp"'\ + $(shell $(PKG_CONFIG) --cflags glib-2.0)\ + $(INC_FLAGS)\ + -MMD -MP +override LDFLAGS += $(shell $(PKG_CONFIG) --libs glib-2.0) + +$(LIBSLIRP): $(OBJS) + +.PHONY: clean + +clean: + rm -r $(OBJS) $(DEPS) $(LIBSLIRP) + +$(BUILD_DIR)/src/%.o: $(ROOT_DIR)/src/%.c + @$(MKDIR_P) $(dir $@) + $(call quiet-command,$(CC) $(CFLAGS) -c -o $@ $<,"CC","$@") + +%.a: + $(call quiet-command,rm -f $@ && $(AR) rcs $@ $^,"AR","$@") + +PKG_CONFIG ?= pkg-config +MKDIR_P ?= mkdir -p +quiet-command-run = $(if $(V),,$(if $2,printf " %-7s %s\n" $2 $3 && ))$1 +quiet-@ = $(if $(V),,@) +quiet-command = $(quiet-@)$(call quiet-command-run,$1,$2,$3) + +print-%: + @echo '$*=$($*)' + +.SUFFIXES: + +-include $(DEPS) -- 2.21.0.rc0.1.g036caf7885
Re: [Qemu-devel] [qemu-s390x] [PATCH v2 1/3] s390x/cpumodel: mepochptff: warn when no mepoch and re-align group init
On 2/12/19 5:04 AM, Cornelia Huck wrote: > On Mon, 11 Feb 2019 20:16:55 -0500 > Collin Walling wrote: > >> The extended PTFF features (qsie, qtoue, stoe, stoue) are dependent >> on the multiple-epoch facility (mepoch). Let's print a warning if these >> features are enabled without mepoch. >> >> While we're at it, let's move the FEAT_GROUP_INIT for mepochptff down >> the s390_feature_groups list so it can be properly indexed with its >> generated S390FeatGroup enum. >> >> Signed-off-by: Collin Walling >> --- >> target/s390x/cpu_features.c | 2 +- >> target/s390x/cpu_models.c | 4 >> 2 files changed, 5 insertions(+), 1 deletion(-) > > Not a comment for this patch (looks fine to me), but more general: > Could you please add a cover letter if you send more than one patch? > It's easy to miss that there are three patches and not two :) > Noted. Thanks for the advice! -- Respectfully, - Collin Walling
Re: [Qemu-devel] [PATCH v3 00/17] block: local qiov helper
On 11.02.2019 6:04, Stefan Hajnoczi wrote: > On Thu, Feb 07, 2019 at 01:24:28PM +0300, Vladimir Sementsov-Ogievskiy wrote: >> v3: > > Will you send a v4 based on Eric's comments or do you want to keep the > series as it is? > I don't really want to resend, and I don't think that open-coding in 16 is a good practice. With my helper we at least have and assertion. I'd be grateful if tiny improvements of 01 are applied in-flight. Anyway, I don't strongly against other Eric's suggestions to be applied, and can resend if it needed.
Re: [Qemu-devel] [PATCH 09/13] block: Add 'runtime_opts' and 'mutable_opts' fields to BlockDriver
Am 17.01.2019 um 16:34 hat Alberto Garcia geschrieben:
> This patch adds two new fields to BlockDriver:
>
>- runtime_opts: list of runtime options for a particular block
> driver. We'll use this list later to detect what options are
> missing when we try to reopen a block device.
>
>- mutable_opts: names of the runtime options that can be reset to
> their default value after a block device has been added. This way
> an option can not be reset by omitting it during reopen unless we
> explicitly allow it.
>
> This also sets runtime_opts (and mutable_opts where appropriate) in
> all drivers that allow reopening.
qcow2 most certainly does support reopening, and is probably one of the
few drivers that actually allow changing things at runtime. Still, it's
missing from this patch.
> Most of those drivers don't actually
> support changing any of their options. If the user specifies a new
> value for an option that can't be changed then we already detect that
> and forbid it (in bdrv_reopen_prepare()). But if the user omits an
> option in order to try to reset it to its default value we need to
> detect that, so we'll use these two new fields for that.
>
> Signed-off-by: Alberto Garcia
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index fd0e88d17a..e680dda86b 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -345,6 +345,13 @@ struct BlockDriver {
>
> /* List of options for creating images, terminated by name == NULL */
> QemuOptsList *create_opts;
> +/* Runtime options for a block device, terminated by name == NULL */
> +QemuOptsList *runtime_opts;
I'm not sure if using a QemuOptsList here is a good idea. Currently, we
use QemuOptsLists for most options, but there are some drivers that use
it only for part of their options, or not at all, using direct QDict
accesses or QAPI objects for the rest.
Especially the part with the QAPI objects is something that I'd hope to
see more in the future, and tying things to QemuOpts might make this
conversion harder.
> +/*
> + * Names of the runtime options that can be reset by omitting
> + * their value on reopen, NULL-terminated.
> + */
> +const char *const *mutable_opts;
>
> /*
> * Returns 0 for completed check, -errno for internal errors.
> diff --git a/block/rbd.c b/block/rbd.c
> index 8a1a9f4b6e..6de6112ce8 100644
> --- a/block/rbd.c
> +++ b/block/rbd.c
> @@ -1231,6 +1231,7 @@ static QemuOptsList qemu_rbd_create_opts = {
> static BlockDriver bdrv_rbd = {
> .format_name= "rbd",
> .instance_size = sizeof(BDRVRBDState),
> +.runtime_opts = _opts,
> .bdrv_parse_filename= qemu_rbd_parse_filename,
> .bdrv_refresh_limits= qemu_rbd_refresh_limits,
> .bdrv_file_open = qemu_rbd_open,
rbd is one of these drivers. In fact, its runtime_opts seems to be
completely unused before this patch.
It has a comemnt 'server.* extracted manually, see qemu_rbd_mon_host()',
but if you compare it to the schema, more options that have been added
since are not covered in runtime_opts.
> diff --git a/block/sheepdog.c b/block/sheepdog.c
> index 90ab43baa4..6dd66d0b99 100644
> --- a/block/sheepdog.c
> +++ b/block/sheepdog.c
> @@ -3288,6 +3288,7 @@ static BlockDriver bdrv_sheepdog_tcp = {
> .bdrv_attach_aio_context = sd_attach_aio_context,
>
> .create_opts = _create_opts,
> +.runtime_opts = _opts,
> };
Sheepdog is another driver where runtime_opts is incomplete (the
'server' struct is missing).
Kevin
