Re: [Qemu-devel] [PATCH] docs: fix COLO architecture diagram
On 11/01/2016 02:25 PM, Hailiang Zhang wrote: Hmm, there are other contents in this file need to be updated, for example, we support blockdev-add command for nbd now, so we can convert the related hmp command to qmp command way. But since we didn't integrate COLO frame with block replication and proxy, It is OK to fix them later. For COLO, the basic capability is still incomplete, but now we got into 'Soft feature freeze' stage, I'm wondering if it is possible at this point to combine COLO frame with proxy and block replication which only needs three or four patches that only touch colo related files ... Or uses still can't test COLO feature in qemu 2.8. Any ideas ? Thanks. I will send some patch about COLO-Proxy combine with COLO-frame in the future. On 2016/11/1 11:38, Zhang Chen wrote: Fix COLO-Proxy part of COLO architecture diagram Signed-off-by: Zhang Chen Reviewed-by: zhanghailiang All such patches can go through trivial branch. Cc: qemu-triv...@nongnu.org I think this patch about COLO architecture, So, I didn't cc qemu-trivial. Thanks Zhang Chen --- docs/COLO-FT.txt | 72 +--- 1 file changed, 37 insertions(+), 35 deletions(-) diff --git a/docs/COLO-FT.txt b/docs/COLO-FT.txt index 6282938..e289be2 100644 --- a/docs/COLO-FT.txt +++ b/docs/COLO-FT.txt @@ -41,41 +41,43 @@ identical responses to all client requests. Once the differences in the outputs are detected between the PVM and SVM, COLO withholds transmission of the outbound packets until it has successfully synchronized the PVM state to the SVM. - Primary Node Secondary Node - ++ +---+ ++ ++ - || | HeartBeat |<->| HeartBeat | || - | Primary VM | +---|---+ +---|+ |Secondary VM| - || | | || - || +---|---+ +---|+ || - || |QEMU +---v+ | |QEMU +v---+| || - || | |Failover| | | |Failover| | || - || | ++ | | ++ | || - || | +---+ | | +---+| || - || | | VM Checkpoint |-->| VM Checkpoint || || - || | +---+ | | +---+| || - || | | | | || - |Requests<---^-->Requests| - |Responses--\ /--|--\ /Responses| - || | | | | | | | | | || - || | +---+ | | | | | | | ++ | || - || | | COLO disk | | | | | | | | | COLO disk | | || - || | | Manager |-|-|--|--|--|->| Manager| | || - || | +|--+ | | | | | | | +---|+ | || - || | || | | | | | | | | || - ++ +--||-|--|--+ +---|--|--|--+ ++ -|| | | | | | - +-+| +--v-v--|--+ +---|--v---+ |+-+ - | VM Monitor || | COLO Proxy | |COLO Proxy | || VM Monitor | - | || |(compare packet) | | (adjust sequence)| || | - +-+| +--|^--+ +--+ |+-+ -|| || - +--|||--+ +-|--+ - | Kernel ||| | | Kernel | | - +--|||--+ +-|--+ -|| || - +--v+ +v|--+ +--+ +v-+ - | Storage | |External Network| | External Network | | Storage| - +---+ ++ +--+ +--+ + Primary Node Secondary Node +++ +---+ ++ ++ +|| | HeartBeat +<->+ HeartBeat | || +| Primary VM | +---+---+ +---++ |Secondary VM| +|| | | |
Re: [Qemu-devel] [PATCH v5 28/33] cputlb: make tlb_flush_by_mmuidx safe for MTTCG
Pranith Kumar writes:
> Hi Alex,
>
> Alex Bennée writes:
>
>> These flushes allow a per-mmuidx granularity to the TLB flushing and are
>> currently only used by the ARM model. As it is possible to hammer the
>> other vCPU threads with flushes (and build up long queues of identical
>> flushes) we extend mechanism used for the global tlb_flush and set a
>> bitmap describing all the pending flushes. The updates are done
>> atomically to avoid corruption of the bitmap but repeating a flush is
>> certainly not a problem.
>>
>> Signed-off-by: Alex Bennée
>
>
>
>>
>> static inline void tlb_flush_entry(CPUTLBEntry *tlb_entry, target_ulong
>> addr)
>> @@ -233,16 +288,50 @@ void tlb_flush_page(CPUState *cpu, target_ulong addr)
>> }
>> }
>>
>> -void tlb_flush_page_by_mmuidx(CPUState *cpu, target_ulong addr, ...)
>> +/* As we are going to hijack the bottom bits of the page address for a
>> + * mmuidx bit mask we need to fail to build if we can't do that
>> + */
>> +QEMU_BUILD_BUG_ON(NB_MMU_MODES > TARGET_PAGE_BITS);
>> +
>
> FYI, this is causing a build error on my system with gcc 6.2.
>
> CC aarch64-softmmu/cputlb.o
> In file included from
> /home/pranith/devops/code/qemu/include/qemu/osdep.h:36:0,
> from /home/pranith/devops/code/qemu/cputlb.c:20:
> /home/pranith/devops/code/qemu/include/exec/cpu-all.h:196:26: error:
> braced-group within expression allowed only inside a function
> #define TARGET_PAGE_BITS ({ assert(target_page_bits_decided); \
> ^
> /home/pranith/devops/code/qemu/include/qemu/compiler.h:89:54: note: in
> definition of macro ‘QEMU_BUILD_BUG_ON’
> typedef char glue(qemu_build_bug_on__,__LINE__)[(x)?-1:1]
> __attribute__((unused));
> ^
> /home/pranith/devops/code/qemu/cputlb.c:293:34: note: in expansion of macro
> ‘TARGET_PAGE_BITS’
> QEMU_BUILD_BUG_ON(NB_MMU_MODES > TARGET_PAGE_BITS);
> ^~~~
> /home/pranith/devops/code/qemu/rules.mak:60: recipe for target 'cputlb.o'
> failed
> make[1]: *** [cputlb.o] Error 1
> Makefile:202: recipe for target 'subdir-aarch64-softmmu' failed
> make: *** [subdir-aarch64-softmmu] Error 2
Odd. I'll look into it. What was you configure string and host architecture?
>
> Thanks,
--
Alex Bennée
Re: [Qemu-devel] [PATCH v10 10/19] vfio iommu: Add blocking notifier to notify DMA_UNMAP
On 11/1/2016 9:15 AM, Dong Jia Shi wrote:
> * Alex Williamson [2016-10-29 08:03:01 -0600]:
>
>> On Sat, 29 Oct 2016 16:07:05 +0530
>> Kirti Wankhede wrote:
>>
>>> On 10/29/2016 2:03 AM, Alex Williamson wrote:
On Sat, 29 Oct 2016 01:32:35 +0530
Kirti Wankhede wrote:
> On 10/28/2016 6:10 PM, Alex Williamson wrote:
>> On Fri, 28 Oct 2016 15:33:58 +0800
>> Jike Song wrote:
>>
>>> ...
+/*
+ * This function finds pfn in domain->external_addr_space->pfn_list
for given
+ * iova range. If pfn exist, notify pfn to registered notifier list.
On
+ * receiving notifier callback, vendor driver should invalidate the
mapping and
+ * call vfio_unpin_pages() to unpin this pfn. With that vfio_pfn for
this pfn
+ * gets removed from rb tree of pfn_list. That re-arranges rb tree,
so while
+ * searching for next vfio_pfn in rb tree, start search from first
node again.
+ * If any vendor driver doesn't unpin that pfn, vfio_pfn would not
get removed
+ * from rb tree and so in next search vfio_pfn would be same as
previous
+ * vfio_pfn. In that case, exit from loop.
+ */
+static void vfio_notifier_call_chain(struct vfio_iommu *iommu,
+ struct vfio_iommu_type1_dma_unmap
*unmap)
+{
+ struct vfio_domain *domain = iommu->external_domain;
+ struct rb_node *n;
+ struct vfio_pfn *vpfn = NULL, *prev_vpfn;
+
+ do {
+ prev_vpfn = vpfn;
+ mutex_lock(&domain->external_addr_space->pfn_list_lock);
+
+ n = rb_first(&domain->external_addr_space->pfn_list);
+
+ for (; n; n = rb_next(n), vpfn = NULL) {
+ vpfn = rb_entry(n, struct vfio_pfn, node);
+
+ if ((vpfn->iova >= unmap->iova) &&
+ (vpfn->iova < unmap->iova + unmap->size))
+ break;
+ }
+
+
mutex_unlock(&domain->external_addr_space->pfn_list_lock);
+
+ /* Notify any listeners about DMA_UNMAP */
+ if (vpfn)
+ blocking_notifier_call_chain(&iommu->notifier,
+
VFIO_IOMMU_NOTIFY_DMA_UNMAP,
+ &vpfn->pfn);
>>>
>>> Hi Kirti,
>>>
>>> The information carried by notifier is only a pfn.
>>>
>>> Since your pin/unpin interfaces design, it's the vendor driver who
>>> should
>>> guarantee pin/unpin same times. To achieve that, the vendor driver must
>>> cache it's iova->pfn mapping on its side, to avoid pinning a same page
>>> for multiple times.
>>>
>>> With the notifier carrying only a pfn, to find the iova by this pfn,
>>> the vendor driver must *also* keep a reverse-mapping. That's a bit
>>> too much.
>>>
>>> Since the vendor could also suffer from IOMMU-compatible problem,
>>> which means a local cache is always helpful, so I'd like to have the
>>> iova carried to the notifier.
>>>
>>> What'd you say?
>>
>> I agree, the pfn is not unique, multiple guest pfns (iovas) might be
>> backed by the same host pfn. DMA_UNMAP calls are based on iova, the
>> notifier through to the vendor driver must be based on the same.
>
> Host pfn should be unique, right?
Let's say a user does a malloc of a single page and does 100 calls to
MAP_DMA populating 100 pages of IOVA space all backed by the same
malloc'd page. This is valid, I have unit tests that do essentially
this. Those will all have the same pfn. The user then does an
UNMAP_DMA to a single one of those IOVA pages. Did the user unmap
everything matching that pfn? Of course not, they only unmapped that
one IOVA page. There is no guarantee of a 1:1 mapping of pfn to IOVA.
UNMAP_DMA works based on IOVA. Invalidation broadcasts to the vendor
driver MUST therefore also work based on IOVA. This is not an academic
problem, address space aliases exist in real VMs, imagine a virtual
IOMMU. Thanks,
>>>
>>>
>>> So struct vfio_iommu_type1_dma_unmap should be passed as argument to
>>> notifier callback:
>>>
>>> if (unmapped && iommu->external_domain)
>>> - vfio_notifier_call_chain(iommu, unmap);
>>> + blocking_notifier_call_chain(&iommu->notifier,
>>> + VFIO_IOMMU_NOTIFY_DMA_UNMAP,
>>> +un
Re: [Qemu-devel] [PATCH v5 28/33] cputlb: make tlb_flush_by_mmuidx safe for MTTCG
On 1 November 2016 at 07:45, Alex Bennée wrote:
>
> Pranith Kumar writes:
>> FYI, this is causing a build error on my system with gcc 6.2.
>>
>> CC aarch64-softmmu/cputlb.o
>> In file included from
>> /home/pranith/devops/code/qemu/include/qemu/osdep.h:36:0,
>> from /home/pranith/devops/code/qemu/cputlb.c:20:
>> /home/pranith/devops/code/qemu/include/exec/cpu-all.h:196:26: error:
>> braced-group within expression allowed only inside a function
>> #define TARGET_PAGE_BITS ({ assert(target_page_bits_decided); \
>> ^
>> /home/pranith/devops/code/qemu/include/qemu/compiler.h:89:54: note: in
>> definition of macro ‘QEMU_BUILD_BUG_ON’
>> typedef char glue(qemu_build_bug_on__,__LINE__)[(x)?-1:1]
>> __attribute__((unused));
>> ^
>> /home/pranith/devops/code/qemu/cputlb.c:293:34: note: in expansion of macro
>> ‘TARGET_PAGE_BITS’
>> QEMU_BUILD_BUG_ON(NB_MMU_MODES > TARGET_PAGE_BITS);
>> ^~~~
>> /home/pranith/devops/code/qemu/rules.mak:60: recipe for target 'cputlb.o'
>> failed
>> make[1]: *** [cputlb.o] Error 1
>> Makefile:202: recipe for target 'subdir-aarch64-softmmu' failed
>> make: *** [subdir-aarch64-softmmu] Error 2
>
> Odd. I'll look into it. What was you configure string and host architecture?
Looks like a clash between the variable-page-size patchset
and your stuff. Now TARGET_PAGE_BITS isn't necessarily
constant you can't use it in a compile time assert like that.
thanks
-- PMM
Re: [Qemu-devel] [PATCH v10 05/19] vfio iommu: Added pin and unpin callback functions to vfio_iommu_driver_ops
On 10/27/2016 05:29 AM, Kirti Wankhede wrote:
> Added two new callback functions to struct vfio_iommu_driver_ops. Backend
> IOMMU module that supports pining and unpinning pages for mdev devices
> should provide these functions.
> Added APIs for pining and unpining pages to VFIO module. These calls back
> into backend iommu module to actually pin and unpin pages.
>
> Signed-off-by: Kirti Wankhede
> Signed-off-by: Neo Jia
> Change-Id: Ia7417723aaae86bec2959ad9ae6c2915ddd340e0
> ---
> drivers/vfio/vfio.c | 92
>
> include/linux/vfio.h | 12 ++-
> 2 files changed, 103 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> index 2e83bdf007fe..28b50ca14c52 100644
> --- a/drivers/vfio/vfio.c
> +++ b/drivers/vfio/vfio.c
> @@ -1799,6 +1799,98 @@ void vfio_info_cap_shift(struct vfio_info_cap *caps,
> size_t offset)
> }
> EXPORT_SYMBOL_GPL(vfio_info_cap_shift);
>
> +
> +/*
> + * Pin a set of guest PFNs and return their associated host PFNs for local
> + * domain only.
> + * @dev [in] : device
> + * @user_pfn [in]: array of user/guest PFNs
> + * @npage [in]: count of array elements
> + * @prot [in] : protection flags
> + * @phys_pfn[out] : array of host PFNs
> + */
Hi Kirti,
Would you also add the documentation what the return value is? It's kind
not clear, and any reason to use long instead of int?
> +long vfio_pin_pages(struct device *dev, unsigned long *user_pfn,
> + long npage, int prot, unsigned long *phys_pfn)
> +{
> + struct vfio_container *container;
> + struct vfio_group *group;
> + struct vfio_iommu_driver *driver;
> + ssize_t ret;
> +
> + if (!dev || !user_pfn || !phys_pfn)
> + return -EINVAL;
> +
> + group = vfio_group_get_from_dev(dev);
> + if (IS_ERR(group))
> + return PTR_ERR(group);
> +
> + ret = vfio_group_add_container_user(group);
> + if (ret)
> + goto err_pin_pages;
> +
> + container = group->container;
> + down_read(&container->group_lock);
> +
> + driver = container->iommu_driver;
> + if (likely(driver && driver->ops->pin_pages))
> + ret = driver->ops->pin_pages(container->iommu_data, user_pfn,
> + npage, prot, phys_pfn);
> + else
> + ret = -EINVAL;
> +
> + up_read(&container->group_lock);
> + vfio_group_try_dissolve_container(group);
> +
> +err_pin_pages:
> + vfio_group_put(group);
> + return ret;
> +
> +}
> +EXPORT_SYMBOL(vfio_pin_pages);
> +
> +/*
> + * Unpin set of host PFNs for local domain only.
> + * @dev [in] : device
> + * @pfn [in] : array of host PFNs to be unpinned.
> + * @npage [in] :count of elements in array, that is number of pages.
> + */
Ditto
--
Thanks,
Jike
> +long vfio_unpin_pages(struct device *dev, unsigned long *pfn, long npage)
> +{
> + struct vfio_container *container;
> + struct vfio_group *group;
> + struct vfio_iommu_driver *driver;
> + ssize_t ret;
> +
> + if (!dev || !pfn)
> + return -EINVAL;
> +
> + group = vfio_group_get_from_dev(dev);
> + if (IS_ERR(group))
> + return PTR_ERR(group);
> +
> + ret = vfio_group_add_container_user(group);
> + if (ret)
> + goto err_unpin_pages;
> +
> + container = group->container;
> + down_read(&container->group_lock);
> +
> + driver = container->iommu_driver;
> + if (likely(driver && driver->ops->unpin_pages))
> + ret = driver->ops->unpin_pages(container->iommu_data, pfn,
> +npage);
> + else
> + ret = -EINVAL;
> +
> + up_read(&container->group_lock);
> + vfio_group_try_dissolve_container(group);
> +
> +err_unpin_pages:
> + vfio_group_put(group);
> + return ret;
> +}
> +EXPORT_SYMBOL(vfio_unpin_pages);
> +
> /**
> * Module/class support
> */
> diff --git a/include/linux/vfio.h b/include/linux/vfio.h
> index 0ecae0b1cd34..0609a2052846 100644
> --- a/include/linux/vfio.h
> +++ b/include/linux/vfio.h
> @@ -75,7 +75,11 @@ struct vfio_iommu_driver_ops {
> struct iommu_group *group);
> void(*detach_group)(void *iommu_data,
> struct iommu_group *group);
> -
> + long(*pin_pages)(void *iommu_data, unsigned long *user_pfn,
> + long npage, int prot,
> + unsigned long *phys_pfn);
> + long(*unpin_pages)(void *iommu_data, unsigned long *pfn,
> +long npage);
> };
>
> extern int vfio_register_iommu_driver(const struct vfio_iommu_driver_ops
> *ops);
> @@ -127,6 +131,12 @@ static inline long vfio_spapr_iommu_eeh_ioctl(struct
> iommu_group *group,
> }
> #endif /* CONFIG_EEH */
>
> +extern long vfio_pin_pages(struct device *dev, unsigned long *user
Re: [Qemu-devel] [PATCH v10 10/19] vfio iommu: Add blocking notifier to notify DMA_UNMAP
* Kirti Wankhede [2016-11-01 13:17:19 +0530]:
>
>
> On 11/1/2016 9:15 AM, Dong Jia Shi wrote:
> > * Alex Williamson [2016-10-29 08:03:01 -0600]:
> >
> >> On Sat, 29 Oct 2016 16:07:05 +0530
> >> Kirti Wankhede wrote:
> >>
> >>> On 10/29/2016 2:03 AM, Alex Williamson wrote:
> On Sat, 29 Oct 2016 01:32:35 +0530
> Kirti Wankhede wrote:
>
> > On 10/28/2016 6:10 PM, Alex Williamson wrote:
> >> On Fri, 28 Oct 2016 15:33:58 +0800
> >> Jike Song wrote:
> >>
> >>> ...
>
> +/*
> + * This function finds pfn in domain->external_addr_space->pfn_list
> for given
> + * iova range. If pfn exist, notify pfn to registered notifier
> list. On
> + * receiving notifier callback, vendor driver should invalidate the
> mapping and
> + * call vfio_unpin_pages() to unpin this pfn. With that vfio_pfn
> for this pfn
> + * gets removed from rb tree of pfn_list. That re-arranges rb tree,
> so while
> + * searching for next vfio_pfn in rb tree, start search from first
> node again.
> + * If any vendor driver doesn't unpin that pfn, vfio_pfn would not
> get removed
> + * from rb tree and so in next search vfio_pfn would be same as
> previous
> + * vfio_pfn. In that case, exit from loop.
> + */
> +static void vfio_notifier_call_chain(struct vfio_iommu *iommu,
> + struct vfio_iommu_type1_dma_unmap
> *unmap)
> +{
> +struct vfio_domain *domain = iommu->external_domain;
> +struct rb_node *n;
> +struct vfio_pfn *vpfn = NULL, *prev_vpfn;
> +
> +do {
> +prev_vpfn = vpfn;
> +mutex_lock(&domain->external_addr_space->pfn_list_lock);
> +
> +n = rb_first(&domain->external_addr_space->pfn_list);
> +
> +for (; n; n = rb_next(n), vpfn = NULL) {
> +vpfn = rb_entry(n, struct vfio_pfn, node);
> +
> +if ((vpfn->iova >= unmap->iova) &&
> +(vpfn->iova < unmap->iova + unmap->size))
> +break;
> +}
> +
> +
> mutex_unlock(&domain->external_addr_space->pfn_list_lock);
> +
> +/* Notify any listeners about DMA_UNMAP */
> +if (vpfn)
> +blocking_notifier_call_chain(&iommu->notifier,
> +
> VFIO_IOMMU_NOTIFY_DMA_UNMAP,
> +&vpfn->pfn);
> >>>
> >>> Hi Kirti,
> >>>
> >>> The information carried by notifier is only a pfn.
> >>>
> >>> Since your pin/unpin interfaces design, it's the vendor driver who
> >>> should
> >>> guarantee pin/unpin same times. To achieve that, the vendor driver
> >>> must
> >>> cache it's iova->pfn mapping on its side, to avoid pinning a same page
> >>> for multiple times.
> >>>
> >>> With the notifier carrying only a pfn, to find the iova by this pfn,
> >>> the vendor driver must *also* keep a reverse-mapping. That's a bit
> >>> too much.
> >>>
> >>> Since the vendor could also suffer from IOMMU-compatible problem,
> >>> which means a local cache is always helpful, so I'd like to have the
> >>> iova carried to the notifier.
> >>>
> >>> What'd you say?
> >>
> >> I agree, the pfn is not unique, multiple guest pfns (iovas) might be
> >> backed by the same host pfn. DMA_UNMAP calls are based on iova, the
> >> notifier through to the vendor driver must be based on the same.
> >
> > Host pfn should be unique, right?
>
> Let's say a user does a malloc of a single page and does 100 calls to
> MAP_DMA populating 100 pages of IOVA space all backed by the same
> malloc'd page. This is valid, I have unit tests that do essentially
> this. Those will all have the same pfn. The user then does an
> UNMAP_DMA to a single one of those IOVA pages. Did the user unmap
> everything matching that pfn? Of course not, they only unmapped that
> one IOVA page. There is no guarantee of a 1:1 mapping of pfn to IOVA.
> UNMAP_DMA works based on IOVA. Invalidation broadcasts to the vendor
> driver MUST therefore also work based on IOVA. This is not an academic
> problem, address space aliases exist in real VMs, imagine a virtual
> IOMMU. Thanks,
>
> >>>
> >>>
> >>> So struct vfio_iommu_type1_dma_unmap should be passed as argument to
> >>> notifier callback:
> >>>
> >>> if (unmapped && iommu->external_domain)
> >>> -
Re: [Qemu-devel] [PATCH v10 00/19] Add Mediated device support
On 10/27/2016 05:29 AM, Kirti Wankhede wrote: > This series adds Mediated device support to Linux host kernel. Purpose > of this series is to provide a common interface for mediated device > management that can be used by different devices. This series introduces > Mdev core module that creates and manages mediated devices, VFIO based > driver for mediated devices that are created by mdev core module and > update VFIO type1 IOMMU module to support pinning & unpinning for mediated > devices. > > What changed in v10? > vfio: > - Split commits in multple individual commits. > - Removed the function added in v9 to get device_api string. > - Defined constant strings in include/uapi/linux/vfio.h that should be used > by >vendor driver for device_api attribute. > > vfio_iommu_type1: > - Fixed accounting when pass through device is unplugged while mdev device >exist in a domain. > - Added blocking notifier to notify DMA_UNMAP to vendor driver to invalidate >mappings. > - Exported APIs to register notifier for DMA_UNMAP action. > > Documentation: > - Added sysfs ABI for mediated device framework. > - Updated Documentation/vfio-mdev/vfio-mediated-device.txt. > - Updated mtty.c with bug fixes. > > Kirti Wankhede (19): > vfio: Mediated device Core driver > vfio: VFIO based driver for Mediated devices > vfio: Rearrange functions to get vfio_group from dev > vfio: Common function to increment container_users > vfio iommu: Added pin and unpin callback functions to > vfio_iommu_driver_ops > vfio iommu type1: Update arguments of vfio_lock_acct > vfio iommu type1: Update argument of vaddr_get_pfn() > vfio iommu type1: Add find_iommu_group() function > vfio iommu type1: Add support for mediated devices > vfio iommu: Add blocking notifier to notify DMA_UNMAP > vfio: Introduce common function to add capabilities > vfio_pci: Update vfio_pci to use vfio_info_add_capability() > vfio: Introduce vfio_set_irqs_validate_and_prepare() > vfio_pci: Updated to use vfio_set_irqs_validate_and_prepare() > vfio_platform: Updated to use vfio_set_irqs_validate_and_prepare() > vfio: Define device_api strings > docs: Add Documentation for Mediated devices > docs: Sysfs ABI for mediated device framework > docs: Sample driver to demonstrate how to use Mediated device > framework. > > Documentation/ABI/testing/sysfs-bus-vfio-mdev| 111 ++ > Documentation/vfio-mdev/Makefile | 13 + > Documentation/vfio-mdev/mtty.c | 1503 > ++ > Documentation/vfio-mdev/vfio-mediated-device.txt | 398 ++ > drivers/vfio/Kconfig |1 + > drivers/vfio/Makefile|1 + > drivers/vfio/mdev/Kconfig| 17 + > drivers/vfio/mdev/Makefile |5 + > drivers/vfio/mdev/mdev_core.c| 384 ++ > drivers/vfio/mdev/mdev_driver.c | 122 ++ > drivers/vfio/mdev/mdev_private.h | 41 + > drivers/vfio/mdev/mdev_sysfs.c | 286 > drivers/vfio/mdev/vfio_mdev.c| 148 +++ > drivers/vfio/pci/vfio_pci.c | 78 +- > drivers/vfio/platform/vfio_platform_common.c | 31 +- > drivers/vfio/vfio.c | 322 - > drivers/vfio/vfio_iommu_type1.c | 808 ++-- > include/linux/mdev.h | 167 +++ > include/linux/vfio.h | 30 +- > include/uapi/linux/vfio.h| 10 + > 20 files changed, 4270 insertions(+), 206 deletions(-) > create mode 100644 Documentation/ABI/testing/sysfs-bus-vfio-mdev > create mode 100644 Documentation/vfio-mdev/Makefile > create mode 100644 Documentation/vfio-mdev/mtty.c > create mode 100644 Documentation/vfio-mdev/vfio-mediated-device.txt > create mode 100644 drivers/vfio/mdev/Kconfig > create mode 100644 drivers/vfio/mdev/Makefile > create mode 100644 drivers/vfio/mdev/mdev_core.c > create mode 100644 drivers/vfio/mdev/mdev_driver.c > create mode 100644 drivers/vfio/mdev/mdev_private.h > create mode 100644 drivers/vfio/mdev/mdev_sysfs.c > create mode 100644 drivers/vfio/mdev/vfio_mdev.c > create mode 100644 include/linux/mdev.h A side note: I rebased KVMGT upon v10, with 2 minor changes: 1, get_user_pages_remote has only 7 args 2, vfio iommu notifier calls vendor callback with iova instead of pfn so far it works pretty well. Thanks! -- Thanks, Jike
Re: [Qemu-devel] [PATCH v5] block/vxhs.c Add support for a new block device type called "vxhs"
On Mon, Oct 31, 2016 at 11:34:58PM -0700, Ashish Mittal wrote: > Source code for the qnio library that this code loads can be downloaded > from: > https://github.com/MittalAshish/libqnio.git Your patch is unable to compile against this, because the qnio_api.h header is using a "struct iovec" type in its APIs, but not including any header to define that type. This causes configure to fail cc -Werror -DHAS_LIBSSH2_SFTP_FSYNC -pthread -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -D_GNU_SOURCE -fPIE -DPIE -m64 -mcx16 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -I/home/berrange/src/external/libqnio/src -Wendif-labels -Wno-shift-negative-value -Wmissing-include-dirs -Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wold-style-declaration -Wold-style-definition -Wtype-limits -fstack-protector-strong -I/usr/include/p11-kit-1 -I/usr/include/libpng16 -I/usr/include/cacard -I/usr/include/nss3 -I/usr/include/nspr4 -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/libusb-1.0 -o config-temp/qemu-conf.exe config-temp/qemu-conf.c -Wl,-z,relro -Wl,-z,now -pie -m64 -g -L/home/berrange/src/external/libqnio/src -lqnio In file included from config-temp/qemu-conf.c:2:0: /home/berrange/src/external/libqnio/src/qnio/qnio_api.h:186:54: error: ‘struct iovec’ declared inside parameter list will not be visible outside of this definition or declaration [-Werror] int32_t iio_writev(void *apictx, int32_t rfd, struct iovec *iov, int iovcnt, ^ /home/berrange/src/external/libqnio/src/qnio/qnio_api.h:189:53: error: ‘struct iovec’ declared inside parameter list will not be visible outside of this definition or declaration [-Werror] int32_t iio_readv(void *apictx, int32_t rfd, struct iovec *iov, int iovcnt, ^ cc1: all warnings being treated as errors Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://entangle-photo.org -o-http://search.cpan.org/~danberr/ :|
Re: [Qemu-devel] [PULL 0/2] x86 and machine queue, 2016-10-31
On 31 October 2016 at 18:30, Eduardo Habkost wrote: > The following changes since commit 6bc56d317f7b5004ea2d89d264bddc8b4d081700: > > Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream-mttcg' into > staging (2016-10-31 15:29:12 +) > > are available in the git repository at: > > git://github.com/ehabkost/qemu.git tags/x86-and-machine-pull-request > > for you to fetch changes up to 83a00f6095a36de2024d6cca57470c4dedfb85ee: > > target-i386: Print warning when mixing [+-]foo and foo=(on|off) (2016-10-31 > 16:20:59 -0200) > > > x86 and machine queue, 2016-10-31 > > > > Eduardo Habkost (2): > tests: Remove unneeded "-vnc none" option > target-i386: Print warning when mixing [+-]foo and foo=(on|off) Applied, thanks. -- PMM
[Qemu-devel] Mentor Summit notes
Valentine and I attended Google Summer of Code Mentor Summit on behalf of QEMU. The event brings together GSoC mentors from all participating organizations. A lot of people benefit from QEMU and told me so at the summit. There are also many who hadn't heard of QEMU before. I gave a lightning talk about Pranith Kumar's MTTCG project since it's a good example of a QEMU project: https://docs.google.com/presentation/d/1kidkMp4k-ak180aI-eZQfWuIEpQoeY-0N-oqt9eZo20/edit?usp=sharing Feedback from Mentor Summit: 1. Pavel Pisa wants to upstream CAN bus support. I asked him to rebase and submit the patches. Please take a look when patches are posted. 2. RTEMS mentioned they sometimes rely on out-of-tree QEMU targets and want to encourage authors to upstream their code. Xilinx was mentioned in particular. 3. Wine brainstormed x86 Windows applications on ARM using qemu-user-x86 + wine (x86). Probably requires multithreaded TCG. Work required to get OpenGL and other native features passed through. Talk to Stefan Dösinger (Wine) if interested. Stefan
Re: [Qemu-devel] [PULL 08/27] hostmem-file: make option 'size' optional
On 10/31/16 20:22 -0200, Eduardo Habkost wrote:
On Mon, Oct 31, 2016 at 03:47:53PM -0400, Paolo Bonzini wrote:
- Original Message -
> From: "Eduardo Habkost"
> To: "Paolo Bonzini"
> Cc: qemu-devel@nongnu.org, "Haozhong Zhang"
> Sent: Monday, October 31, 2016 7:20:10 PM
> Subject: Re: [Qemu-devel] [PULL 08/27] hostmem-file: make option 'size'
optional
>
> On Mon, Oct 31, 2016 at 03:37:24PM +0100, Paolo Bonzini wrote:
> [...]
> > @@ -1309,21 +1317,27 @@ static void *file_ram_alloc(RAMBlock *block,
> >
> > file_size = get_file_size(fd);
> >
> > -if (memory < block->page_size) {
> > +if (!mem_size && file_size > 0) {
> > +mem_size = file_size;
> > +memory_region_set_size(block->mr, mem_size);
> > +}
> > +
> > +if (mem_size < block->page_size) {
> > error_setg(errp, "memory size 0x" RAM_ADDR_FMT " must be equal to
> > "
> > "or larger than page size 0x%zx",
> > - memory, block->page_size);
> > + mem_size, block->page_size);
> > goto error;
> > }
> >
> > -if (file_size > 0 && file_size < memory) {
> > +if (file_size > 0 && file_size < mem_size) {
> > error_setg(errp, "backing store %s size %"PRId64
> > " does not match 'size' option %"PRIu64,
> > - path, file_size, memory);
> > + path, file_size, mem_size);
> > goto error;
> > }
> >
> > -memory = ROUND_UP(memory, block->page_size);
> > +mem_size = ROUND_UP(mem_size, block->page_size);
> > +*memory = mem_size;
>
> I suggested not touching *memory unless it was zero, and setting
> it to the memory region size, not the rounded-up size. Haozhong
> said this was going to be changed.
>
> This will have the side-effect of setting block->used_length and
> block->max_length to the rounded up size in
> qemu_ram_alloc_from_file() (instead of the original memory region
> size). I don't know what could be the consequences of that.
>
> This patch also skip HOST_PAGE_ALIGN-ing mem_size after getting
> the file size, which would be different from the behavior when
> size is specified explicitly. (And I also don't know the
> consequences of that)
>
> Considering that this pull request failed to build, I suggest
> waiting for a new version from Haozhong.
Yes, I'll drop these three patches.
I believe you can keep the other two (as long as the build error
is fixed). I was already going to include them in my pull
request, but removed them.
I'm a little confused. Do I need to send a following patch to fix this
build error? I was going to send a new version of the entire patch
series.
Thanks,
Haozhong
Re: [Qemu-devel] [PATCH v2] qxl: Only emit QXL_INTERRUPT_CLIENT_MONITORS_CONFIG on config changes
> This commit makes sure that we only emit > QXL_INTERRUPT_CLIENT_MONITORS_CONFIG when there are actual configuration > changes the guest should act on. Added to vga queue. thanks, Gerd
[Qemu-devel] [PATCH] virtio-gpu: fix information leak in getting capset info dispatch
From: Li Qiang
In virgl_cmd_get_capset_info dispatch function, the 'resp' hasn't
been full initialized before writing to the guest. This will leak
the 'resp.padding' and 'resp.hdr.padding' fieds to the guest. This
patch fix this issue.
Signed-off-by: Li Qiang
---
hw/display/virtio-gpu-3d.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/display/virtio-gpu-3d.c b/hw/display/virtio-gpu-3d.c
index 758d33a..23f39de 100644
--- a/hw/display/virtio-gpu-3d.c
+++ b/hw/display/virtio-gpu-3d.c
@@ -347,6 +347,7 @@ static void virgl_cmd_get_capset_info(VirtIOGPU *g,
VIRTIO_GPU_FILL_CMD(info);
+memset(&resp, 0, sizeof(resp));
if (info.capset_index == 0) {
resp.capset_id = VIRTIO_GPU_CAPSET_VIRGL;
virgl_renderer_get_cap_set(resp.capset_id,
--
1.8.3.1
Re: [Qemu-devel] [PATCH 2/4] block/curl: Fix return value from curl_read_cb
I've long since lost all context on this patch, so I'll trust you :) Thanks for the ping, Matt On Wed, Oct 26, 2016 at 10:17 AM, Kevin Wolf wrote: > Am 25.10.2016 um 20:37 hat Eric Blake geschrieben: > > On 10/24/2016 09:54 PM, Max Reitz wrote: > > > While commit 38bbc0a580f9f10570b1d1b5d3e92f0e6feb2970 is correct in > that > > > the callback is supposed to return the number of bytes handled; what it > > > does not mention is that libcurl will throw an error if the callback > did > > > not "handle" all of the data passed to it. > > > > > > Therefore, if the callback receives some data that it cannot handle > > > (either because the receive buffer has not been set up yet or because > it > > > would not fit into the receive buffer) and we have to ignore it, we > > > still have to report that the data has been handled. > > > > > > Obviously, this should not happen normally. But it does happen at least > > > for FTP connections where some data (that we do not expect) may be > > > generated when the connection is established. > > > > Just to make sure, we aren't losing data by reporting this value, but > > merely letting curl know that our callback has "dealt" with the data, so > > that we don't error out, in order to get a second chance at the same > > data later on? > > > > Reviewed-by: Eric Blake > > > > But given that it undoes 38bbc0a, I'd rather that it gets reviewed by > > Matthew and/or tested by Richard. > > In that case, I guess we should CC them. (Hereby done.) > > Kevin > -- Matthew Booth Red Hat Engineering, Virtualisation Team Phone: +442070094448 (UK)
Re: [Qemu-devel] [PATCH 1/6] tests/docker/Makefile.include: fix diff-index call
Fam Zheng writes: > On Fri, 10/28 17:33, Alex Bennée wrote: >> The whole thing is wrapped inside a call quiet-command as well as being >> the actual call taking a --quiet argument so the redirect is >> superfluous. For reasons I have yet to determine this also seems to be >> causing the source preparation step to skip stashing work tree stuff. >> >> Signed-off-by: Alex Bennée >> >> --- >> TODO: >> - properly understand the failure > > Yep, I don't see the bug on my machine (Fedora 24, git 2.7.4). What about the > removed "--"? Does that make a difference for you? Nope - it seems to be the &>/dev/null that triggers the problem - but only in the Makefile. Running the commands from the command line works as expected. However why do we need the redirect here anyway considering the call quiet-command? > >> --- >> tests/docker/Makefile.include | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include >> index 3f15d5a..d91e28b 100644 >> --- a/tests/docker/Makefile.include >> +++ b/tests/docker/Makefile.include >> @@ -20,7 +20,7 @@ IMAGES ?= % >> # Make archive from git repo $1 to tar.gz $2 >> make-archive-maybe = $(if $(wildcard $1/*), \ >> $(call quiet-command, \ >> -(cd $1; if git diff-index --quiet HEAD -- &>/dev/null; then \ >> +(cd $1; if git diff-index --quiet HEAD; then \ >> git archive -1 HEAD --format=tar.gz; \ >> else \ >> git archive -1 $$(git stash create) --format=tar.gz; \ >> -- >> 2.10.1 >> >> -- Alex Bennée
Re: [Qemu-devel] [PATCH 3/3] Move getting XWindow ID from baum driver to graphical backend
Hi, > typedef void QemuConsoleConfigListener(void); I think we should also pass the console which has changed: QemuConsoleConfigListener(QemuConsole *con); > qemu_console_config_add_listener(QemuConsoleConfigListener listener); > qemu_console_config_remove_listener(QemuConsoleConfigListener listener); Otherwise this approach looks good to me. cheers, Gerd
Re: [Qemu-devel] [PATCH 1/4] console: add API to get underlying gui window ID
> int qemu_console_get_width(QemuConsole *con, int fallback);
> int qemu_console_get_height(QemuConsole *con, int fallback);
> +/* Return the low-level window id for the first graphical console */
> +int qemu_graphic_console_get_window_id(void);
> +void qemu_console_set_window_id(int index, int window_id);
Both qemu_console_{set,get}_window_id should have a QemuConsole *con
argument, like the other ones.
There is also no reason to limit this interface to graphic consoles.
baum.c can use qemu_console_lookup_by_index() and
qemu_console_is_graphic() to implement this logic (sorry, missed this on
the first review).
cheers,
Gerd
Re: [Qemu-devel] [RFC 1/2] linux-headers: update
On 29 October 2016 at 22:10, Alexander Graf wrote: > This patch updates the Linux headers to include the in-progress user > space ARM timer patches. It is explicitly RFC, as the patches are not > merged yet. > --- Is there a cover letter email for this series ? thanks -- PMM
Re: [Qemu-devel] [PATCH 1/1] migration: fix compiler warning on uninitialized variable
On 31 October 2016 at 21:50, Jeff Cody wrote:
> Some older GCC versions (e.g. 4.4.7) report a warning on an
> uninitialized variable for 'request', even though all possible code
> paths that reference 'request' will be initialized. To appease
> these versions, initialize the variable to 0.
>
> Reported-by: Mark Cave-Ayland
> Signed-off-by: Jeff Cody
> ---
> migration/colo.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/migration/colo.c b/migration/colo.c
> index e7224b8..93c85c5 100644
> --- a/migration/colo.c
> +++ b/migration/colo.c
> @@ -439,7 +439,7 @@ void *colo_process_incoming_thread(void *opaque)
> }
>
> while (mis->state == MIGRATION_STATUS_COLO) {
> -int request;
> +int request = 0;
>
> colo_wait_handle_message(mis->from_src_file, &request, &local_err);
> if (local_err) {
> --
> 2.7.4
Thanks, applied to master as a buildfix.
-- PMM
Re: [Qemu-devel] [PULL 00/14] Block patches for 2.8
On 31 October 2016 at 19:22, Jeff Cody wrote: > The following changes since commit 6bc56d317f7b5004ea2d89d264bddc8b4d081700: > > Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream-mttcg' into > staging (2016-10-31 15:29:12 +) > > are available in the git repository at: > > g...@github.com:codyprime/qemu-kvm-jtc.git tags/block-pull-request > > for you to fetch changes up to 4f5afc7abe382fc22bdf0ca67537d545e1044f2c: > > blockjobs: fix documentation (2016-10-31 15:22:08 -0400) > > > Block patches for 2.8 > Sorry, this doesn't apply cleanly to master, and my attempt to fix up the conflict didn't seem to work. Please can you rebase and resend? thanks -- PMM
Re: [Qemu-devel] [PULL 37/47] nvdimm acpi: introduce fit buffer
On Tue, 1 Nov 2016 11:30:46 +0800
Xiao Guangrong wrote:
> On 10/31/2016 07:09 PM, Igor Mammedov wrote:
> > On Mon, 31 Oct 2016 17:52:24 +0800
> > Xiao Guangrong wrote:
> >
> >> On 10/31/2016 05:45 PM, Igor Mammedov wrote:
> >>> On Sun, 30 Oct 2016 23:25:05 +0200
> >>> "Michael S. Tsirkin" wrote:
> >>>
> From: Xiao Guangrong
>
> The buffer is used to save the FIT info for all the presented nvdimm
> devices which is updated after the nvdimm device is plugged or
> unplugged. In the later patch, it will be used to construct NVDIMM
> ACPI _FIT method which reflects the presented nvdimm devices after
> nvdimm hotplug
>
> As FIT buffer can not completely mapped into guest address space,
> OSPM will exit to QEMU multiple times, however, there is the race
> condition - FIT may be changed during these multiple exits, so that
> some rules are introduced:
> 1) the user should hold the @lock to access the buffer and
> > Could you explain why lock is needed?
>
> Yes. These are two things need to be synced between QEMU and OSPM:
> - fit buffer. QEMU products it and VM will consume it at the same time.
> - dirty indicator. QEMU sets it and it will be cleared by OSPM, that means.
>both sides will modify it.
I understand why 'dirty indicator' is necessary but
could you describe a concrete call flows in detail
that would cause concurrent access and need extra
lock protection.
Note that there is global lock (dubbed BQL) in QEMU,
which is taken every time guest accesses IO port or
QMP/monitor control event happens.
> 2) mark @dirty whenever the buffer is updated.
>
> @dirty is cleared for the first time OSPM gets fit buffer, if
> dirty is detected in the later access, OSPM will restart the
> access
>
> As fit should be updated after nvdimm device is successfully realized
> so that a new hotplug callback, post_hotplug, is introduced
>
> Signed-off-by: Xiao Guangrong
> Reviewed-by: Michael S. Tsirkin
> Signed-off-by: Michael S. Tsirkin
> ---
> include/hw/hotplug.h| 10 +
> include/hw/mem/nvdimm.h | 26 +-
> hw/acpi/nvdimm.c| 59
> +++--
> hw/core/hotplug.c | 11 +
> hw/core/qdev.c | 20 +
> hw/i386/acpi-build.c| 2 +-
> hw/i386/pc.c| 19
> 7 files changed, 124 insertions(+), 23 deletions(-)
>
> diff --git a/include/hw/hotplug.h b/include/hw/hotplug.h
> index c0db869..10ca5b6 100644
> --- a/include/hw/hotplug.h
> +++ b/include/hw/hotplug.h
> @@ -47,6 +47,7 @@ typedef void (*hotplug_fn)(HotplugHandler
> *plug_handler,
> * @parent: Opaque parent interface.
> * @pre_plug: pre plug callback called at start of device.realize(true)
> * @plug: plug callback called at end of device.realize(true).
> + * @post_pug: post plug callback called after device is successfully
> plugged.
> >>> this doesn't seem to be necessary,
> >>> why hotplug_handler_plug() isn't sufficient?
> >>
> >> At the point of hotplug_handler_plug(), the device is not realize
> >> (realized == 0),
> >> however, nvdimm_get_plugged_device_list() works on realized nvdimm
> >> devices.
> >
> > I suggest instead of adding redundant hook, to reuse hotplug_handler_plug()
> > which is called after device::realize() successfully completed and amend
> > nvdimm_plugged_device_list() as follow:
> >
> > diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
> > index e486128..c6d8cbb 100644
> > --- a/hw/acpi/nvdimm.c
> > +++ b/hw/acpi/nvdimm.c
> > @@ -38,11 +38,7 @@ static int nvdimm_plugged_device_list(Object *obj, void
> > *opaque)
> > GSList **list = opaque;
> >
> > if (object_dynamic_cast(obj, TYPE_NVDIMM)) {
> > -DeviceState *dev = DEVICE(obj);
> > -
> > -if (dev->realized) { /* only realized NVDIMMs matter */
> > -*list = g_slist_append(*list, DEVICE(obj));
> > -}
> > +*list = g_slist_append(*list, obj);
> > }
> >
> > object_child_foreach(obj, nvdimm_plugged_device_list, opaque);
> >
> > that should count not only already present nvdimms but also the one that's
> > being hotplugged.
>
> It is not good as the device which failed to initialize
See device_set_realized()
[...]
if (dc->realize) {
dc->realize(dev, &local_err);
}
if (local_err != NULL) {
goto fail;
}
Re: [Qemu-devel] [PATCH 1/6] tests/docker/Makefile.include: fix diff-index call
On Tue, 11/01 10:02, Alex Bennée wrote: > > Fam Zheng writes: > > > On Fri, 10/28 17:33, Alex Bennée wrote: > >> The whole thing is wrapped inside a call quiet-command as well as being > >> the actual call taking a --quiet argument so the redirect is > >> superfluous. For reasons I have yet to determine this also seems to be > >> causing the source preparation step to skip stashing work tree stuff. > >> > >> Signed-off-by: Alex Bennée > >> > >> --- > >> TODO: > >> - properly understand the failure > > > > Yep, I don't see the bug on my machine (Fedora 24, git 2.7.4). What about > > the > > removed "--"? Does that make a difference for you? > > Nope - it seems to be the &>/dev/null that triggers the problem - but > only in the Makefile. Running the commands from the command line works > as expected. > > However why do we need the redirect here anyway considering the call > quiet-command? It's to force suppressing output, while quiet-command only suppresses the echoing of the command itself. Since we have "--quiet" here, does "2>/dev/null" work for you? Fam > > > > >> --- > >> tests/docker/Makefile.include | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include > >> index 3f15d5a..d91e28b 100644 > >> --- a/tests/docker/Makefile.include > >> +++ b/tests/docker/Makefile.include > >> @@ -20,7 +20,7 @@ IMAGES ?= % > >> # Make archive from git repo $1 to tar.gz $2 > >> make-archive-maybe = $(if $(wildcard $1/*), \ > >>$(call quiet-command, \ > >> - (cd $1; if git diff-index --quiet HEAD -- &>/dev/null; then \ > >> + (cd $1; if git diff-index --quiet HEAD; then \ > >>git archive -1 HEAD --format=tar.gz; \ > >>else \ > >>git archive -1 $$(git stash create) --format=tar.gz; \ > >> -- > >> 2.10.1 > >> > >> > > > -- > Alex Bennée
[Qemu-devel] [PATCH] virtio-gpu: fix memory leak in update_cursor_data_virgl
From: Li Qiang
In update_cursor_data_virgl function, if the 'width'/ 'height'
is not equal to current cursor's width/height it will return
without free the 'data' allocated previously. This will lead
a memory leak issue. This patch fix this issue.
Signed-off-by: Li Qiang
---
hw/display/virtio-gpu.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 60bce94..5f32e1a 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -84,6 +84,7 @@ static void update_cursor_data_virgl(VirtIOGPU *g,
if (width != s->current_cursor->width ||
height != s->current_cursor->height) {
+free(data);
return;
}
--
1.8.3.1
Re: [Qemu-devel] [PATCH] virtio-gpu: fix information leak in getting capset info dispatch
On Di, 2016-11-01 at 02:53 -0700, Li Qiang wrote: > From: Li Qiang > > In virgl_cmd_get_capset_info dispatch function, the 'resp' hasn't > been full initialized before writing to the guest. This will leak > the 'resp.padding' and 'resp.hdr.padding' fieds to the guest. This > patch fix this issue. > > Signed-off-by: Li Qiang Added to vga queue. thanks, Gerd
Re: [Qemu-devel] [PATCH] virtio-gpu: fix memory leak in update_cursor_data_virgl
On Di, 2016-11-01 at 04:06 -0700, Li Qiang wrote:
> From: Li Qiang
>
> In update_cursor_data_virgl function, if the 'width'/ 'height'
> is not equal to current cursor's width/height it will return
> without free the 'data' allocated previously. This will lead
> a memory leak issue. This patch fix this issue.
>
> Signed-off-by: Li Qiang
> ---
> hw/display/virtio-gpu.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> index 60bce94..5f32e1a 100644
> --- a/hw/display/virtio-gpu.c
> +++ b/hw/display/virtio-gpu.c
> @@ -84,6 +84,7 @@ static void update_cursor_data_virgl(VirtIOGPU *g,
>
> if (width != s->current_cursor->width ||
> height != s->current_cursor->height) {
> +free(data);
> return;
> }
>
Added to vga patch queue.
thanks,
Gerd
Re: [Qemu-devel] [PATCH] virtio-gpu: fix memory leak in update_cursor_data_virgl
Hi
On Tue, Nov 1, 2016 at 2:07 PM Li Qiang wrote:
> From: Li Qiang
>
> In update_cursor_data_virgl function, if the 'width'/ 'height'
> is not equal to current cursor's width/height it will return
> without free the 'data' allocated previously. This will lead
> a memory leak issue. This patch fix this issue.
>
> Signed-off-by: Li Qiang
>
Reviewed-by: Marc-André Lureau
---
> hw/display/virtio-gpu.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> index 60bce94..5f32e1a 100644
> --- a/hw/display/virtio-gpu.c
> +++ b/hw/display/virtio-gpu.c
> @@ -84,6 +84,7 @@ static void update_cursor_data_virgl(VirtIOGPU *g,
>
> if (width != s->current_cursor->width ||
> height != s->current_cursor->height) {
> +free(data);
> return;
> }
>
> --
> 1.8.3.1
>
>
> --
Marc-André Lureau
Re: [Qemu-devel] [PULL] Update OpenBIOS images
On 31 October 2016 at 20:25, Mark Cave-Ayland wrote: > Hi Peter, > > Here are the OpenBIOS updates for the 2.8 release. Please pull. > > > ATB, > > Mark. > > > The following changes since commit e80b4b8fb6babce7dcc91ea9ddeecbc351fd4646: > > Merge remote-tracking branch > 'remotes/awilliam/tags/vfio-updates-20161031.0' into staging (2016-10-31 > 18:19:06 +) > > are available in the git repository at: > > > https://github.com/mcayland/qemu.git tags/qemu-openbios-signed > > for you to fetch changes up to 625ed4be4b00ad4469814786535508e1ade8351e: > > Update OpenBIOS images to 1dc4f16 built from submodule. (2016-10-31 > 20:01:25 +) > > > Update OpenBIOS images > > > Mark Cave-Ayland (1): > Update OpenBIOS images to 1dc4f16 built from submodule. > > pc-bios/openbios-ppc | Bin 750840 -> 750840 bytes > pc-bios/openbios-sparc32 | Bin 382048 -> 382048 bytes > pc-bios/openbios-sparc64 | Bin 1593424 -> 1593408 bytes > roms/openbios|2 +- > 4 files changed, 1 insertion(+), 1 deletion(-) Applied, thanks. -- PMM
Re: [Qemu-devel] [RFC 2/2] ARM: KVM: Enable in-kernel timers with user space gic
On 29 October 2016 at 22:10, Alexander Graf wrote:
> When running with KVM enabled, you can choose between emulating the
> gic in kernel or user space. If the kernel supports in-kernel virtualization
> of the interrupt controller, it will default to that. If not, if will
> default to user space emulation.
>
> Unfortunately when running in user mode gic emulation, we miss out on
> timer events which are only available from kernel space. This patch leverages
> the new kernel/user space pending line synchronization for those timer events.
>
> Signed-off-by: Alexander Graf
> ---
> hw/arm/virt.c| 10 ++
> target-arm/cpu.h | 3 +++
> target-arm/kvm.c | 19 +++
> 3 files changed, 32 insertions(+)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 070bbf8..8ac81e3 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -622,6 +622,16 @@ static void create_gic(VirtBoardInfo *vbi, qemu_irq
> *pic, int type,
> } else if (type == 2) {
> create_v2m(vbi, pic);
> }
> +
> +#ifdef CONFIG_KVM
> +if (kvm_enabled() && !kvm_irqchip_in_kernel()) {
> +if (!kvm_check_extension(kvm_state, KVM_CAP_ARM_TIMER)) {
> +error_report("KVM with user space irqchip only works when the "
> + "host kernel supports KVM_CAP_ARM_TIMER");
> +exit(1);
> +}
> +}
> +#endif
I think this belongs somewhere in target-arm/kvm.c rather
than in hw/arm/virt.c -- it's not the only board model that
supports KVM.
> }
>
> static void create_uart(const VirtBoardInfo *vbi, qemu_irq *pic, int uart,
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 19d967b..7686082 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -659,6 +659,9 @@ struct ARMCPU {
>
> ARMELChangeHook *el_change_hook;
> void *el_change_hook_opaque;
> +
> +/* Used to synchronize KVM and QEMU timer levels */
> +uint8_t timer_irq_level;
> };
>
> static inline ARMCPU *arm_env_get_cpu(CPUARMState *env)
> diff --git a/target-arm/kvm.c b/target-arm/kvm.c
> index c00b94e..0d8b642 100644
> --- a/target-arm/kvm.c
> +++ b/target-arm/kvm.c
> @@ -527,6 +527,25 @@ void kvm_arch_pre_run(CPUState *cs, struct kvm_run *run)
>
> MemTxAttrs kvm_arch_post_run(CPUState *cs, struct kvm_run *run)
> {
> +ARMCPU *cpu;
> +
> +if (kvm_irqchip_in_kernel()) {
> +/*
> + * We only need to sync timer states with user-space interrupt
> + * controllers, so return early and save cycles if we don't.
> + */
> +return MEMTXATTRS_UNSPECIFIED;
> +}
> +
> +cpu = ARM_CPU(cs);
> +
> +/* Synchronize our internal vtimer irq line with the kvm one */
> +if (run->s.regs.timer_irq_level != cpu->timer_irq_level) {
> +qemu_set_irq(ARM_CPU(cs)->gt_timer_outputs[GTIMER_VIRT],
You just set up a local variable, so you don't need to inline "ARM_CPU(cs)".
> + run->s.regs.timer_irq_level & KVM_ARM_TIMER_VTIMER);
This is setting a bear trap for the person who comes along later
to add the next interrupt, because the level argument to qemu_set_irq()
should be 0 or 1. That happens to be true for the KVM_ARM_TIMER_VTIMER
bit but won't be for the cut-n-pasted version with the next bit name...
> +cpu->timer_irq_level = run->s.regs.timer_irq_level;
> +}
> +
> return MEMTXATTRS_UNSPECIFIED;
> }
Does this code do the right thing across a vcpu reset or
a full-system reset?
>
> --
> 1.8.5.6
thanks
-- PMM
Re: [Qemu-devel] [PATCH 1/6] tests/docker/Makefile.include: fix diff-index call
Fam Zheng writes: > On Tue, 11/01 10:02, Alex Bennée wrote: >> >> Fam Zheng writes: >> >> > On Fri, 10/28 17:33, Alex Bennée wrote: >> >> The whole thing is wrapped inside a call quiet-command as well as being >> >> the actual call taking a --quiet argument so the redirect is >> >> superfluous. For reasons I have yet to determine this also seems to be >> >> causing the source preparation step to skip stashing work tree stuff. >> >> >> >> Signed-off-by: Alex Bennée >> >> >> >> --- >> >> TODO: >> >> - properly understand the failure >> > >> > Yep, I don't see the bug on my machine (Fedora 24, git 2.7.4). What about >> > the >> > removed "--"? Does that make a difference for you? >> >> Nope - it seems to be the &>/dev/null that triggers the problem - but >> only in the Makefile. Running the commands from the command line works >> as expected. >> >> However why do we need the redirect here anyway considering the call >> quiet-command? > > It's to force suppressing output, while quiet-command only suppresses the > echoing of the command itself. Since we have "--quiet" here, does > "2>/dev/null" > work for you? That works! - (cd $1; if git diff-index --quiet HEAD -- &>/dev/null; then \ + (cd $1; if git diff-index --quiet HEAD -- 2>/dev/null; then \ > > Fam > >> >> > >> >> --- >> >> tests/docker/Makefile.include | 2 +- >> >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> >> >> diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include >> >> index 3f15d5a..d91e28b 100644 >> >> --- a/tests/docker/Makefile.include >> >> +++ b/tests/docker/Makefile.include >> >> @@ -20,7 +20,7 @@ IMAGES ?= % >> >> # Make archive from git repo $1 to tar.gz $2 >> >> make-archive-maybe = $(if $(wildcard $1/*), \ >> >> $(call quiet-command, \ >> >> - (cd $1; if git diff-index --quiet HEAD -- &>/dev/null; then \ >> >> + (cd $1; if git diff-index --quiet HEAD; then \ >> >> git archive -1 HEAD --format=tar.gz; \ >> >> else \ >> >> git archive -1 $$(git stash create) --format=tar.gz; \ >> >> -- >> >> 2.10.1 >> >> >> >> >> >> >> -- >> Alex Bennée -- Alex Bennée
Re: [Qemu-devel] [PULL v2 for-2.8 00/15] target-sparc updates
On 31 October 2016 at 20:50, Richard Henderson wrote: > V2 with a workaround for win32 namespace pollution. Whee! > Only resending patch 09/15, wherein the change lies. > > > r~ > > > The following changes since commit 4178c782f85530d261058abdccc734aa9b7c89ca: > > Merge remote-tracking branch > 'remotes/pmaydell/tags/pull-target-arm-20161028' into staging (2016-10-31 > 11:12:02 +) > > are available in the git repository at: > > git://github.com/rth7680/qemu.git tags/pull-sparc-20161031-2 > > for you to fetch changes up to 5a7267b6a9e94c264ca77a7ca5a239e70dac81da: > > target-sparc: Use tcg_gen_atomic_cmpxchg_tl (2016-10-31 14:46:48 -0600) > > > target-sparc updates for atomics and alignment > Applied, thanks. -- PMM
Re: [Qemu-devel] [PULL 00/14] Block patches for 2.8
On Tue, Nov 01, 2016 at 10:23:59AM +, Peter Maydell wrote: > On 31 October 2016 at 19:22, Jeff Cody wrote: > > The following changes since commit 6bc56d317f7b5004ea2d89d264bddc8b4d081700: > > > > Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream-mttcg' > > into staging (2016-10-31 15:29:12 +) > > > > are available in the git repository at: > > > > g...@github.com:codyprime/qemu-kvm-jtc.git tags/block-pull-request > > > > for you to fetch changes up to 4f5afc7abe382fc22bdf0ca67537d545e1044f2c: > > > > blockjobs: fix documentation (2016-10-31 15:22:08 -0400) > > > > > > Block patches for 2.8 > > > > Sorry, this doesn't apply cleanly to master, and my attempt > to fix up the conflict didn't seem to work. Please can you > rebase and resend? > Yes, sure. Will send v2 shortly.
[Qemu-devel] [PULL v2 for-2.8 5/7] 9pfs: xattrcreate requires non-opened fids
The xattrcreate operation only makes sense on a freshly cloned fid
actually, since any open state would be leaked because of the fid_type
change. This is indeed what the linux kernel client does:
fid = clone_fid(fid);
[...]
retval = p9_client_xattrcreate(fid, name, value_len, flags);
This patch also reverts commit ff55e94d23ae since we are sure that a fid
with type P9_FID_NONE doesn't have a previously allocated xattr.
Signed-off-by: Greg Kurz
---
hw/9pfs/9p.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 27af0072599a..547f3b558079 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -3272,6 +3272,11 @@ static void coroutine_fn v9fs_xattrcreate(void *opaque)
err = -EINVAL;
goto out_nofid;
}
+if (file_fidp->fid_type != P9_FID_NONE) {
+err = -EINVAL;
+goto out_put_fid;
+}
+
/* Make the file fid point to xattr */
xattr_fidp = file_fidp;
xattr_fidp->fid_type = P9_FID_XATTR;
@@ -3281,9 +3286,9 @@ static void coroutine_fn v9fs_xattrcreate(void *opaque)
xattr_fidp->fs.xattr.flags = flags;
v9fs_string_init(&xattr_fidp->fs.xattr.name);
v9fs_string_copy(&xattr_fidp->fs.xattr.name, &name);
-g_free(xattr_fidp->fs.xattr.value);
xattr_fidp->fs.xattr.value = g_malloc0(size);
err = offset;
+out_put_fid:
put_fid(pdu, file_fidp);
out_nofid:
pdu_complete(pdu, err);
--
2.5.5
[Qemu-devel] [PULL v2 for-2.8 0/7] 9p patches for 2.8 20161030
The following changes since commit 02ba9265e8d65f24d0cdca158d96e0b0451f6b71: migration: fix compiler warning on uninitialized variable (2016-11-01 09:31:53 +) are available in the git repository at: https://github.com/gkurz/qemu.git tags/for-upstream for you to fetch changes up to 79decce35b4d769fa878b048ab1a7b3e9045c9c6: 9pfs: drop excessive error message from virtfs_reset() (2016-11-01 12:03:03 +0100) This pull request mostly contains some more fixes to prevent buggy guests from breaking QEMU. v2: - added missing SoB tags - rebased Greg Kurz (4): 9pfs: limit xattr size in xattrcreate 9pfs: xattrcreate requires non-opened fids 9pfs: don't BUG_ON() if fid is already opened 9pfs: drop excessive error message from virtfs_reset() Li Qiang (3): 9pfs: add xattrwalk_fid field in V9fsXattr struct 9pfs: convert 'len/copied_len' field in V9fsXattr to the type of uint64_t 9pfs: fix integer overflow issue in xattr read/write hw/9pfs/9p.c | 80 +--- hw/9pfs/9p.h | 5 ++-- hw/9pfs/trace-events | 2 +- 3 files changed, 49 insertions(+), 38 deletions(-) -- 2.5.5
[Qemu-devel] [PULL v2 for-2.8 1/7] 9pfs: add xattrwalk_fid field in V9fsXattr struct
From: Li Qiang
Currently, 9pfs sets the 'copied_len' field in V9fsXattr
to -1 to tag xattr walk fid. As the 'copied_len' is also
used to account for copied bytes, this may make confusion. This patch
add a bool 'xattrwalk_fid' to tag the xattr walk fid.
Suggested-by: Greg Kurz
Signed-off-by: Li Qiang
Reviewed-by: Greg Kurz
Signed-off-by: Greg Kurz
---
hw/9pfs/9p.c | 7 ---
hw/9pfs/9p.h | 1 +
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index e88cf257a2b9..ab18ef2adf32 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -325,7 +325,7 @@ static int coroutine_fn v9fs_xattr_fid_clunk(V9fsPDU *pdu,
V9fsFidState *fidp)
{
int retval = 0;
-if (fidp->fs.xattr.copied_len == -1) {
+if (fidp->fs.xattr.xattrwalk_fid) {
/* getxattr/listxattr fid */
goto free_value;
}
@@ -3190,7 +3190,7 @@ static void coroutine_fn v9fs_xattrwalk(void *opaque)
*/
xattr_fidp->fs.xattr.len = size;
xattr_fidp->fid_type = P9_FID_XATTR;
-xattr_fidp->fs.xattr.copied_len = -1;
+xattr_fidp->fs.xattr.xattrwalk_fid = true;
if (size) {
xattr_fidp->fs.xattr.value = g_malloc(size);
err = v9fs_co_llistxattr(pdu, &xattr_fidp->path,
@@ -3223,7 +3223,7 @@ static void coroutine_fn v9fs_xattrwalk(void *opaque)
*/
xattr_fidp->fs.xattr.len = size;
xattr_fidp->fid_type = P9_FID_XATTR;
-xattr_fidp->fs.xattr.copied_len = -1;
+xattr_fidp->fs.xattr.xattrwalk_fid = true;
if (size) {
xattr_fidp->fs.xattr.value = g_malloc(size);
err = v9fs_co_lgetxattr(pdu, &xattr_fidp->path,
@@ -3279,6 +3279,7 @@ static void coroutine_fn v9fs_xattrcreate(void *opaque)
xattr_fidp = file_fidp;
xattr_fidp->fid_type = P9_FID_XATTR;
xattr_fidp->fs.xattr.copied_len = 0;
+xattr_fidp->fs.xattr.xattrwalk_fid = false;
xattr_fidp->fs.xattr.len = size;
xattr_fidp->fs.xattr.flags = flags;
v9fs_string_init(&xattr_fidp->fs.xattr.name);
diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h
index 2523a445f81f..48065cc22169 100644
--- a/hw/9pfs/9p.h
+++ b/hw/9pfs/9p.h
@@ -164,6 +164,7 @@ typedef struct V9fsXattr
void *value;
V9fsString name;
int flags;
+bool xattrwalk_fid;
} V9fsXattr;
typedef struct V9fsDir {
--
2.5.5
[Qemu-devel] [PULL v2 for-2.8 3/7] 9pfs: fix integer overflow issue in xattr read/write
From: Li Qiang
The v9fs_xattr_read() and v9fs_xattr_write() are passed a guest
originated offset: they must ensure this offset does not go beyond
the size of the extended attribute that was set in v9fs_xattrcreate().
Unfortunately, the current code implement these checks with unsafe
calculations on 32 and 64 bit values, which may allow a malicious
guest to cause OOB access anyway.
Fix this by comparing the offset and the xattr size, which are
both uint64_t, before trying to compute the effective number of bytes
to read or write.
Suggested-by: Greg Kurz
Signed-off-by: Li Qiang
Reviewed-by: Greg Kurz
Reviewed-By: Guido Günther
Signed-off-by: Greg Kurz
---
hw/9pfs/9p.c | 32
1 file changed, 12 insertions(+), 20 deletions(-)
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index ab18ef2adf32..7705ead4b2ac 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -1637,20 +1637,17 @@ static int v9fs_xattr_read(V9fsState *s, V9fsPDU *pdu,
V9fsFidState *fidp,
{
ssize_t err;
size_t offset = 7;
-int read_count;
-int64_t xattr_len;
+uint64_t read_count;
V9fsVirtioState *v = container_of(s, V9fsVirtioState, state);
VirtQueueElement *elem = v->elems[pdu->idx];
-xattr_len = fidp->fs.xattr.len;
-read_count = xattr_len - off;
+if (fidp->fs.xattr.len < off) {
+read_count = 0;
+} else {
+read_count = fidp->fs.xattr.len - off;
+}
if (read_count > max_count) {
read_count = max_count;
-} else if (read_count < 0) {
-/*
- * read beyond XATTR value
- */
-read_count = 0;
}
err = pdu_marshal(pdu, offset, "d", read_count);
if (err < 0) {
@@ -1979,23 +1976,18 @@ static int v9fs_xattr_write(V9fsState *s, V9fsPDU *pdu,
V9fsFidState *fidp,
{
int i, to_copy;
ssize_t err = 0;
-int write_count;
-int64_t xattr_len;
+uint64_t write_count;
size_t offset = 7;
-xattr_len = fidp->fs.xattr.len;
-write_count = xattr_len - off;
-if (write_count > count) {
-write_count = count;
-} else if (write_count < 0) {
-/*
- * write beyond XATTR value len specified in
- * xattrcreate
- */
+if (fidp->fs.xattr.len < off) {
err = -ENOSPC;
goto out;
}
+write_count = fidp->fs.xattr.len - off;
+if (write_count > count) {
+write_count = count;
+}
err = pdu_marshal(pdu, offset, "d", write_count);
if (err < 0) {
return err;
--
2.5.5
[Qemu-devel] [PULL v2 for-2.8 7/7] 9pfs: drop excessive error message from virtfs_reset()
The virtfs_reset() function is called either when the virtio-9p device
gets reset, or when the client starts a new 9P session. In both cases,
if it finds fids from a previous session, the following is printed in
the monitor:
9pfs:virtfs_reset: One or more uncluncked fids found during reset
For example, if a linux guest with a mounted 9P share is reset from the
monitor with system_reset, the message will be printed. This is excessive
since these fids are now clunked and the state is clean.
Signed-off-by: Greg Kurz
Reviewed-by: Eric Blake
---
hw/9pfs/9p.c | 7 +--
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 1050b89ec720..aea7e9d39206 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -535,7 +535,7 @@ static int coroutine_fn v9fs_mark_fids_unreclaim(V9fsPDU
*pdu, V9fsPath *path)
static void coroutine_fn virtfs_reset(V9fsPDU *pdu)
{
V9fsState *s = pdu->s;
-V9fsFidState *fidp = NULL;
+V9fsFidState *fidp;
/* Free all fids */
while (s->fid_list) {
@@ -548,11 +548,6 @@ static void coroutine_fn virtfs_reset(V9fsPDU *pdu)
free_fid(pdu, fidp);
}
}
-if (fidp) {
-/* One or more unclunked fids found... */
-error_report("9pfs:%s: One or more uncluncked fids "
- "found during reset", __func__);
-}
}
#define P9_QID_TYPE_DIR 0x80
--
2.5.5
[Qemu-devel] [PULL v2 for-2.8 2/7] 9pfs: convert 'len/copied_len' field in V9fsXattr to the type of uint64_t
From: Li Qiang
The 'len' in V9fsXattr comes from the 'size' argument in setxattr()
function in guest. The setxattr() function's declaration is this:
int setxattr(const char *path, const char *name,
const void *value, size_t size, int flags);
and 'size' is treated as u64 in linux kernel client code:
int p9_client_xattrcreate(struct p9_fid *fid, const char *name,
u64 attr_size, int flags)
So the 'len' should have an type of 'uint64_t'.
The 'copied_len' in V9fsXattr is used to account for copied bytes, it
should also have an type of 'uint64_t'.
Suggested-by: Greg Kurz
Signed-off-by: Li Qiang
Reviewed-by: Greg Kurz
Signed-off-by: Greg Kurz
---
hw/9pfs/9p.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h
index 48065cc22169..3976b7fe3dcd 100644
--- a/hw/9pfs/9p.h
+++ b/hw/9pfs/9p.h
@@ -159,8 +159,8 @@ typedef struct V9fsConf
typedef struct V9fsXattr
{
-int64_t copied_len;
-int64_t len;
+uint64_t copied_len;
+uint64_t len;
void *value;
V9fsString name;
int flags;
--
2.5.5
[Qemu-devel] [PULL v2 for-2.8 4/7] 9pfs: limit xattr size in xattrcreate
We shouldn't allow guests to create extended attribute with arbitrary sizes.
On linux hosts, the limit is XATTR_SIZE_MAX. Let's use it.
Signed-off-by: Greg Kurz
---
hw/9pfs/9p.c | 7 ++-
hw/9pfs/trace-events | 2 +-
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 7705ead4b2ac..27af0072599a 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -3247,7 +3247,7 @@ static void coroutine_fn v9fs_xattrcreate(void *opaque)
{
int flags;
int32_t fid;
-int64_t size;
+uint64_t size;
ssize_t err = 0;
V9fsString name;
size_t offset = 7;
@@ -3262,6 +3262,11 @@ static void coroutine_fn v9fs_xattrcreate(void *opaque)
}
trace_v9fs_xattrcreate(pdu->tag, pdu->id, fid, name.data, size, flags);
+if (size > XATTR_SIZE_MAX) {
+err = -E2BIG;
+goto out_nofid;
+}
+
file_fidp = get_fid(pdu, fid);
if (file_fidp == NULL) {
err = -EINVAL;
diff --git a/hw/9pfs/trace-events b/hw/9pfs/trace-events
index 48d3d8abedb5..fb4de3d465ab 100644
--- a/hw/9pfs/trace-events
+++ b/hw/9pfs/trace-events
@@ -42,6 +42,6 @@ v9fs_mkdir(uint16_t tag, uint8_t id, int32_t fid, char* name,
int mode, uint32_t
v9fs_mkdir_return(uint16_t tag, uint8_t id, int8_t type, int32_t version,
int64_t path, int err) "tag %u id %u qid={type %d version %d path %"PRId64"}
err %d"
v9fs_xattrwalk(uint16_t tag, uint8_t id, int32_t fid, int32_t newfid, char*
name) "tag %d id %d fid %d newfid %d name %s"
v9fs_xattrwalk_return(uint16_t tag, uint8_t id, int64_t size) "tag %d id %d
size %"PRId64
-v9fs_xattrcreate(uint16_t tag, uint8_t id, int32_t fid, char* name, int64_t
size, int flags) "tag %d id %d fid %d name %s size %"PRId64" flags %d"
+v9fs_xattrcreate(uint16_t tag, uint8_t id, int32_t fid, char* name, uint64_t
size, int flags) "tag %d id %d fid %d name %s size %"PRIu64" flags %d"
v9fs_readlink(uint16_t tag, uint8_t id, int32_t fid) "tag %d id %d fid %d"
v9fs_readlink_return(uint16_t tag, uint8_t id, char* target) "tag %d id %d
name %s"
--
2.5.5
[Qemu-devel] [PULL v2 for-2.8 6/7] 9pfs: don't BUG_ON() if fid is already opened
A buggy or malicious guest could pass the id of an already opened fid and
cause QEMU to abort. Let's return EINVAL to the guest instead.
Signed-off-by: Greg Kurz
Reviewed-by: Eric Blake
---
hw/9pfs/9p.c | 20
1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 547f3b558079..1050b89ec720 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -1361,7 +1361,10 @@ static void coroutine_fn v9fs_walk(void *opaque)
memcpy(&qids[name_idx], &qid, sizeof(qid));
}
if (fid == newfid) {
-BUG_ON(fidp->fid_type != P9_FID_NONE);
+if (fidp->fid_type != P9_FID_NONE) {
+err = -EINVAL;
+goto out;
+}
v9fs_path_copy(&fidp->path, &path);
} else {
newfidp = alloc_fid(s, newfid);
@@ -1443,7 +1446,10 @@ static void coroutine_fn v9fs_open(void *opaque)
err = -ENOENT;
goto out_nofid;
}
-BUG_ON(fidp->fid_type != P9_FID_NONE);
+if (fidp->fid_type != P9_FID_NONE) {
+err = -EINVAL;
+goto out;
+}
err = v9fs_co_lstat(pdu, &fidp->path, &stbuf);
if (err < 0) {
@@ -2540,7 +2546,10 @@ static int coroutine_fn v9fs_complete_rename(V9fsPDU
*pdu, V9fsFidState *fidp,
err = -ENOENT;
goto out_nofid;
}
-BUG_ON(dirfidp->fid_type != P9_FID_NONE);
+if (fidp->fid_type != P9_FID_NONE) {
+err = -EINVAL;
+goto out;
+}
v9fs_co_name_to_path(pdu, &dirfidp->path, name->data, &new_path);
} else {
old_name = fidp->path.data;
@@ -2612,7 +2621,10 @@ static void coroutine_fn v9fs_rename(void *opaque)
err = -ENOENT;
goto out_nofid;
}
-BUG_ON(fidp->fid_type != P9_FID_NONE);
+if (fidp->fid_type != P9_FID_NONE) {
+err = -EINVAL;
+goto out;
+}
/* if fs driver is not path based, return EOPNOTSUPP */
if (!(pdu->s->ctx.export_flags & V9FS_PATHNAME_FSCONTEXT)) {
err = -EOPNOTSUPP;
--
2.5.5
[Qemu-devel] [PULL v2 for-2.8 5/9] tcg: Add tcg_gen_mulsu2_{i32, i64, tl}
This multiply has one signed input and one unsigned input,
producing the full double-width result.
Signed-off-by: Richard Henderson
Message-Id: <1475011433-24456-2-git-send-email-...@twiddle.net>
---
tcg/tcg-op.c | 43 +++
tcg/tcg-op.h | 4
2 files changed, 47 insertions(+)
diff --git a/tcg/tcg-op.c b/tcg/tcg-op.c
index bb2bfee..4d125df 100644
--- a/tcg/tcg-op.c
+++ b/tcg/tcg-op.c
@@ -678,6 +678,33 @@ void tcg_gen_muls2_i32(TCGv_i32 rl, TCGv_i32 rh, TCGv_i32
arg1, TCGv_i32 arg2)
}
}
+void tcg_gen_mulsu2_i32(TCGv_i32 rl, TCGv_i32 rh, TCGv_i32 arg1, TCGv_i32 arg2)
+{
+if (TCG_TARGET_REG_BITS == 32) {
+TCGv_i32 t0 = tcg_temp_new_i32();
+TCGv_i32 t1 = tcg_temp_new_i32();
+TCGv_i32 t2 = tcg_temp_new_i32();
+tcg_gen_mulu2_i32(t0, t1, arg1, arg2);
+/* Adjust for negative input for the signed arg1. */
+tcg_gen_sari_i32(t2, arg1, 31);
+tcg_gen_and_i32(t2, t2, arg2);
+tcg_gen_sub_i32(rh, t1, t2);
+tcg_gen_mov_i32(rl, t0);
+tcg_temp_free_i32(t0);
+tcg_temp_free_i32(t1);
+tcg_temp_free_i32(t2);
+} else {
+TCGv_i64 t0 = tcg_temp_new_i64();
+TCGv_i64 t1 = tcg_temp_new_i64();
+tcg_gen_ext_i32_i64(t0, arg1);
+tcg_gen_extu_i32_i64(t1, arg2);
+tcg_gen_mul_i64(t0, t0, t1);
+tcg_gen_extr_i64_i32(rl, rh, t0);
+tcg_temp_free_i64(t0);
+tcg_temp_free_i64(t1);
+}
+}
+
void tcg_gen_ext8s_i32(TCGv_i32 ret, TCGv_i32 arg)
{
if (TCG_TARGET_HAS_ext8s_i32) {
@@ -1748,6 +1775,22 @@ void tcg_gen_muls2_i64(TCGv_i64 rl, TCGv_i64 rh,
TCGv_i64 arg1, TCGv_i64 arg2)
}
}
+void tcg_gen_mulsu2_i64(TCGv_i64 rl, TCGv_i64 rh, TCGv_i64 arg1, TCGv_i64 arg2)
+{
+TCGv_i64 t0 = tcg_temp_new_i64();
+TCGv_i64 t1 = tcg_temp_new_i64();
+TCGv_i64 t2 = tcg_temp_new_i64();
+tcg_gen_mulu2_i64(t0, t1, arg1, arg2);
+/* Adjust for negative input for the signed arg1. */
+tcg_gen_sari_i64(t2, arg1, 63);
+tcg_gen_and_i64(t2, t2, arg2);
+tcg_gen_sub_i64(rh, t1, t2);
+tcg_gen_mov_i64(rl, t0);
+tcg_temp_free_i64(t0);
+tcg_temp_free_i64(t1);
+tcg_temp_free_i64(t2);
+}
+
/* Size changing operations. */
void tcg_gen_extrl_i64_i32(TCGv_i32 ret, TCGv_i64 arg)
diff --git a/tcg/tcg-op.h b/tcg/tcg-op.h
index 89b59e8..6d044b7 100644
--- a/tcg/tcg-op.h
+++ b/tcg/tcg-op.h
@@ -306,6 +306,7 @@ void tcg_gen_sub2_i32(TCGv_i32 rl, TCGv_i32 rh, TCGv_i32 al,
TCGv_i32 ah, TCGv_i32 bl, TCGv_i32 bh);
void tcg_gen_mulu2_i32(TCGv_i32 rl, TCGv_i32 rh, TCGv_i32 arg1, TCGv_i32 arg2);
void tcg_gen_muls2_i32(TCGv_i32 rl, TCGv_i32 rh, TCGv_i32 arg1, TCGv_i32 arg2);
+void tcg_gen_mulsu2_i32(TCGv_i32 rl, TCGv_i32 rh, TCGv_i32 arg1, TCGv_i32
arg2);
void tcg_gen_ext8s_i32(TCGv_i32 ret, TCGv_i32 arg);
void tcg_gen_ext16s_i32(TCGv_i32 ret, TCGv_i32 arg);
void tcg_gen_ext8u_i32(TCGv_i32 ret, TCGv_i32 arg);
@@ -482,6 +483,7 @@ void tcg_gen_sub2_i64(TCGv_i64 rl, TCGv_i64 rh, TCGv_i64 al,
TCGv_i64 ah, TCGv_i64 bl, TCGv_i64 bh);
void tcg_gen_mulu2_i64(TCGv_i64 rl, TCGv_i64 rh, TCGv_i64 arg1, TCGv_i64 arg2);
void tcg_gen_muls2_i64(TCGv_i64 rl, TCGv_i64 rh, TCGv_i64 arg1, TCGv_i64 arg2);
+void tcg_gen_mulsu2_i64(TCGv_i64 rl, TCGv_i64 rh, TCGv_i64 arg1, TCGv_i64
arg2);
void tcg_gen_not_i64(TCGv_i64 ret, TCGv_i64 arg);
void tcg_gen_ext8s_i64(TCGv_i64 ret, TCGv_i64 arg);
void tcg_gen_ext16s_i64(TCGv_i64 ret, TCGv_i64 arg);
@@ -956,6 +958,7 @@ void tcg_gen_atomic_xor_fetch_i64(TCGv_i64, TCGv, TCGv_i64,
TCGArg, TCGMemOp);
#define tcg_gen_sub2_tl tcg_gen_sub2_i64
#define tcg_gen_mulu2_tl tcg_gen_mulu2_i64
#define tcg_gen_muls2_tl tcg_gen_muls2_i64
+#define tcg_gen_mulsu2_tl tcg_gen_mulsu2_i64
#define tcg_gen_atomic_cmpxchg_tl tcg_gen_atomic_cmpxchg_i64
#define tcg_gen_atomic_xchg_tl tcg_gen_atomic_xchg_i64
#define tcg_gen_atomic_fetch_add_tl tcg_gen_atomic_fetch_add_i64
@@ -1043,6 +1046,7 @@ void tcg_gen_atomic_xor_fetch_i64(TCGv_i64, TCGv,
TCGv_i64, TCGArg, TCGMemOp);
#define tcg_gen_sub2_tl tcg_gen_sub2_i32
#define tcg_gen_mulu2_tl tcg_gen_mulu2_i32
#define tcg_gen_muls2_tl tcg_gen_muls2_i32
+#define tcg_gen_mulsu2_tl tcg_gen_mulsu2_i32
#define tcg_gen_atomic_cmpxchg_tl tcg_gen_atomic_cmpxchg_i32
#define tcg_gen_atomic_xchg_tl tcg_gen_atomic_xchg_i32
#define tcg_gen_atomic_fetch_add_tl tcg_gen_atomic_fetch_add_i32
--
2.7.4
[Qemu-devel] [PULL v2 for-2.8 1/9] target-cris: Do not dump cpu state with -d in_asm
Dumping cpu state is what -d cpu is for.
Reviewed-by: Edgar E. Iglesias
Reviewed-by: Alex Bennée
Signed-off-by: Richard Henderson
---
target-cris/translate.c | 25 ++---
1 file changed, 2 insertions(+), 23 deletions(-)
diff --git a/target-cris/translate.c b/target-cris/translate.c
index b5ab0a5..8d4c864 100644
--- a/target-cris/translate.c
+++ b/target-cris/translate.c
@@ -3135,29 +3135,6 @@ void gen_intermediate_code(CPUCRISState *env, struct
TranslationBlock *tb)
dc->cpustate_changed = 0;
-if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)) {
-qemu_log(
-"pc=%x %x flg=%" PRIx64 " bt=%x ds=%u ccs=%x\n"
-"pid=%x usp=%x\n"
-"%x.%x.%x.%x\n"
-"%x.%x.%x.%x\n"
-"%x.%x.%x.%x\n"
-"%x.%x.%x.%x\n",
-dc->pc, dc->ppc,
-(uint64_t)tb->flags,
-env->btarget, (unsigned)tb->flags & 7,
-env->pregs[PR_CCS],
-env->pregs[PR_PID], env->pregs[PR_USP],
-env->regs[0], env->regs[1], env->regs[2], env->regs[3],
-env->regs[4], env->regs[5], env->regs[6], env->regs[7],
-env->regs[8], env->regs[9],
-env->regs[10], env->regs[11],
-env->regs[12], env->regs[13],
-env->regs[14], env->regs[15]);
-qemu_log("--\n");
-qemu_log("IN: %s\n", lookup_symbol(pc_start));
-}
-
next_page_start = (pc_start & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE;
num_insns = 0;
max_insns = tb->cflags & CF_COUNT_MASK;
@@ -3313,6 +3290,8 @@ void gen_intermediate_code(CPUCRISState *env, struct
TranslationBlock *tb)
#if !DISAS_CRIS
if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)
&& qemu_log_in_addr_range(pc_start)) {
+qemu_log("--\n");
+qemu_log("IN: %s\n", lookup_symbol(pc_start));
log_target_disas(cs, pc_start, dc->pc - pc_start,
env->pregs[PR_VR]);
qemu_log("\nisize=%d osize=%d\n",
--
2.7.4
[Qemu-devel] [PULL v2 for-2.8 2/9] target-microblaze: Do not dump cpu state with -d in_asm
Dumping cpu state is what -d cpu is for.
Reviewed-by: Edgar E. Iglesias
Signed-off-by: Richard Henderson
---
target-microblaze/translate.c | 9 +
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/target-microblaze/translate.c b/target-microblaze/translate.c
index 80098ec..5a4a8b9 100644
--- a/target-microblaze/translate.c
+++ b/target-microblaze/translate.c
@@ -1670,13 +1670,6 @@ void gen_intermediate_code(CPUMBState *env, struct
TranslationBlock *tb)
cpu_abort(cs, "Microblaze: unaligned PC=%x\n", pc_start);
}
-if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)) {
-#if !SIM_COMPAT
-qemu_log("--\n");
-log_cpu_state(CPU(cpu), 0);
-#endif
-}
-
next_page_start = (pc_start & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE;
num_insns = 0;
max_insns = tb->cflags & CF_COUNT_MASK;
@@ -1820,7 +1813,7 @@ void gen_intermediate_code(CPUMBState *env, struct
TranslationBlock *tb)
#if !SIM_COMPAT
if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)
&& qemu_log_in_addr_range(pc_start)) {
-qemu_log("\n");
+qemu_log("--\n");
#if DISAS_GNU
log_target_disas(cs, pc_start, dc->pc - pc_start, 0);
#endif
--
2.7.4
[Qemu-devel] [PULL v2 for-2.8 0/9] tcg queued patches
V2, now with more windows workarounds. I'll note that I have not
been 100% successful in actually building a mingw64 image. But
at least the error Peter saw with v1 is fixed.
I'll report on the other mingw64 failures under separate cover.
r~
The following changes since commit e80b4b8fb6babce7dcc91ea9ddeecbc351fd4646:
Merge remote-tracking branch 'remotes/awilliam/tags/vfio-updates-20161031.0'
into staging (2016-10-31 18:19:06 +)
are available in the git repository at:
git://github.com/rth7680/qemu.git tags/pull-tcg-20161101
for you to fetch changes up to 13110e2d41d0b9d77edea5b23e1a808c86eb22cb:
tcg: correct 32-bit tcg_gen_ld8s_i64 sign-extension (2016-10-31 21:36:29
-0600)
tcg queued patches
Joseph Myers (1):
tcg: correct 32-bit tcg_gen_ld8s_i64 sign-extension
Peter Maydell (1):
tcg/tcg.h: Improve documentation of TCGv_i32 etc types
Pranith Kumar (1):
MAINTAINERS: Update PPC status and maintainer
Richard Henderson (6):
target-cris: Do not dump cpu state with -d in_asm
target-microblaze: Do not dump cpu state with -d in_asm
target-openrisc: Do not dump cpu state with -d in_asm
log: Add locking to large logging blocks
tcg: Add tcg_gen_mulsu2_{i32,i64,tl}
target-microblaze: Cleanup dec_mul
MAINTAINERS | 4 +--
cpu-exec.c| 2 ++
exec.c| 2 ++
include/qemu/log.h| 16 ++
include/sysemu/os-posix.h | 12
include/sysemu/os-win32.h | 11 +++
target-alpha/translate.c | 2 ++
target-arm/translate-a64.c| 2 ++
target-arm/translate.c| 2 ++
target-cris/translate.c | 27 +++-
target-i386/translate.c | 4 +++
target-lm32/translate.c | 2 ++
target-m68k/translate.c | 2 ++
target-microblaze/translate.c | 72 ---
target-mips/translate.c | 2 ++
target-openrisc/translate.c | 9 +++---
target-ppc/translate.c| 2 ++
target-s390x/translate.c | 2 ++
target-sh4/translate.c| 2 ++
target-sparc/translate.c | 2 ++
target-tilegx/translate.c | 6 +++-
target-tricore/translate.c| 2 ++
target-unicore32/translate.c | 2 ++
target-xtensa/translate.c | 2 ++
tcg/tcg-op.c | 45 ++-
tcg/tcg-op.h | 4 +++
tcg/tcg.c | 8 +
tcg/tcg.h | 38 ++-
translate-all.c | 2 ++
29 files changed, 188 insertions(+), 100 deletions(-)
[Qemu-devel] [PULL v2 for-2.8 7/9] MAINTAINERS: Update PPC status and maintainer
From: Pranith Kumar Richard agreed to make odd fixes to PPC tcg parts[1]. This patch makes the change. [1] https://lists.gnu.org/archive/html/qemu-ppc/2016-03/msg00657.html Signed-off-by: Pranith Kumar Signed-off-by: Richard Henderson --- MAINTAINERS | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index 3fecf45..653f52e 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1504,8 +1504,8 @@ F: tcg/mips/ F: disas/mips.c PPC -M: Vassili Karpov (malc) -S: Maintained +M: Richard Henderson +S: Odd Fixes F: tcg/ppc/ F: disas/ppc.c -- 2.7.4
[Qemu-devel] [PULL v2 for-2.8 3/9] target-openrisc: Do not dump cpu state with -d in_asm
Dumping cpu state is what -d cpu is for.
Cc: Jia Liu
Signed-off-by: Richard Henderson
---
target-openrisc/translate.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/target-openrisc/translate.c b/target-openrisc/translate.c
index 28c9446..a4625f9 100644
--- a/target-openrisc/translate.c
+++ b/target-openrisc/translate.c
@@ -1651,10 +1651,6 @@ void gen_intermediate_code(CPUOpenRISCState *env, struct
TranslationBlock *tb)
dc->synced_flags = dc->tb_flags = tb->flags;
dc->delayed_branch = !!(dc->tb_flags & D_FLAG);
dc->singlestep_enabled = cs->singlestep_enabled;
-if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)) {
-qemu_log("-\n");
-log_cpu_state(CPU(cpu), 0);
-}
next_page_start = (pc_start & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE;
num_insns = 0;
@@ -1754,7 +1750,8 @@ void gen_intermediate_code(CPUOpenRISCState *env, struct
TranslationBlock *tb)
#ifdef DEBUG_DISAS
if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)
&& qemu_log_in_addr_range(pc_start)) {
-qemu_log("\n");
+qemu_log("\n");
+qemu_log("IN: %s\n", lookup_symbol(pc_start));
log_target_disas(cs, pc_start, dc->pc - pc_start, 0);
qemu_log("\nisize=%d osize=%d\n",
dc->pc - pc_start, tcg_op_buf_count());
--
2.7.4
[Qemu-devel] [PULL v2 for-2.8 6/9] target-microblaze: Cleanup dec_mul
Use tcg_gen_mul_tl for muli and mul instructions.
Use tcg_gen_muls2_tl for mulh instruction.
Use tcg_gen_mulu2_tl for mulhu instruction.
Use tcg_gen_mulsu2_tl for mulhsu instruction.
Note that this last fixes a bug, in that mulhsu was
previously treating both operands as signed, instead
of treating rb as unsigned.
Tested-by: Edgar E. Iglesias
Reviewed-by: Edgar E. Iglesias
Signed-off-by: Richard Henderson
Message-Id: <1475011433-24456-3-git-send-email-...@twiddle.net>
---
target-microblaze/translate.c | 61 +++
1 file changed, 9 insertions(+), 52 deletions(-)
diff --git a/target-microblaze/translate.c b/target-microblaze/translate.c
index 5274191..de2090a 100644
--- a/target-microblaze/translate.c
+++ b/target-microblaze/translate.c
@@ -581,50 +581,10 @@ static void dec_msr(DisasContext *dc)
}
}
-/* 64-bit signed mul, lower result in d and upper in d2. */
-static void t_gen_muls(TCGv d, TCGv d2, TCGv a, TCGv b)
-{
-TCGv_i64 t0, t1;
-
-t0 = tcg_temp_new_i64();
-t1 = tcg_temp_new_i64();
-
-tcg_gen_ext_i32_i64(t0, a);
-tcg_gen_ext_i32_i64(t1, b);
-tcg_gen_mul_i64(t0, t0, t1);
-
-tcg_gen_extrl_i64_i32(d, t0);
-tcg_gen_shri_i64(t0, t0, 32);
-tcg_gen_extrl_i64_i32(d2, t0);
-
-tcg_temp_free_i64(t0);
-tcg_temp_free_i64(t1);
-}
-
-/* 64-bit unsigned muls, lower result in d and upper in d2. */
-static void t_gen_mulu(TCGv d, TCGv d2, TCGv a, TCGv b)
-{
-TCGv_i64 t0, t1;
-
-t0 = tcg_temp_new_i64();
-t1 = tcg_temp_new_i64();
-
-tcg_gen_extu_i32_i64(t0, a);
-tcg_gen_extu_i32_i64(t1, b);
-tcg_gen_mul_i64(t0, t0, t1);
-
-tcg_gen_extrl_i64_i32(d, t0);
-tcg_gen_shri_i64(t0, t0, 32);
-tcg_gen_extrl_i64_i32(d2, t0);
-
-tcg_temp_free_i64(t0);
-tcg_temp_free_i64(t1);
-}
-
/* Multiplier unit. */
static void dec_mul(DisasContext *dc)
{
-TCGv d[2];
+TCGv tmp;
unsigned int subcode;
if ((dc->tb_flags & MSR_EE_FLAG)
@@ -636,13 +596,11 @@ static void dec_mul(DisasContext *dc)
}
subcode = dc->imm & 3;
-d[0] = tcg_temp_new();
-d[1] = tcg_temp_new();
if (dc->type_b) {
LOG_DIS("muli r%d r%d %x\n", dc->rd, dc->ra, dc->imm);
-t_gen_mulu(cpu_R[dc->rd], d[1], cpu_R[dc->ra], *(dec_alu_op_b(dc)));
-goto done;
+tcg_gen_mul_tl(cpu_R[dc->rd], cpu_R[dc->ra], *(dec_alu_op_b(dc)));
+return;
}
/* mulh, mulhsu and mulhu are not available if C_USE_HW_MUL is < 2. */
@@ -651,30 +609,29 @@ static void dec_mul(DisasContext *dc)
/* nop??? */
}
+tmp = tcg_temp_new();
switch (subcode) {
case 0:
LOG_DIS("mul r%d r%d r%d\n", dc->rd, dc->ra, dc->rb);
-t_gen_mulu(cpu_R[dc->rd], d[1], cpu_R[dc->ra], cpu_R[dc->rb]);
+tcg_gen_mul_tl(cpu_R[dc->rd], cpu_R[dc->ra], cpu_R[dc->rb]);
break;
case 1:
LOG_DIS("mulh r%d r%d r%d\n", dc->rd, dc->ra, dc->rb);
-t_gen_muls(d[0], cpu_R[dc->rd], cpu_R[dc->ra], cpu_R[dc->rb]);
+tcg_gen_muls2_tl(tmp, cpu_R[dc->rd], cpu_R[dc->ra], cpu_R[dc->rb]);
break;
case 2:
LOG_DIS("mulhsu r%d r%d r%d\n", dc->rd, dc->ra, dc->rb);
-t_gen_muls(d[0], cpu_R[dc->rd], cpu_R[dc->ra], cpu_R[dc->rb]);
+tcg_gen_mulsu2_tl(tmp, cpu_R[dc->rd], cpu_R[dc->ra],
cpu_R[dc->rb]);
break;
case 3:
LOG_DIS("mulhu r%d r%d r%d\n", dc->rd, dc->ra, dc->rb);
-t_gen_mulu(d[0], cpu_R[dc->rd], cpu_R[dc->ra], cpu_R[dc->rb]);
+tcg_gen_mulu2_tl(tmp, cpu_R[dc->rd], cpu_R[dc->ra], cpu_R[dc->rb]);
break;
default:
cpu_abort(CPU(dc->cpu), "unknown MUL insn %x\n", subcode);
break;
}
-done:
-tcg_temp_free(d[0]);
-tcg_temp_free(d[1]);
+tcg_temp_free(tmp);
}
/* Div unit. */
--
2.7.4
[Qemu-devel] [PULL v2 for-2.8 9/9] tcg: correct 32-bit tcg_gen_ld8s_i64 sign-extension
From: Joseph Myers
The version of tcg_gen_ld8s_i64 for 32-bit systems does a load into
the low part of the return value - then attempts a sign extension into
the high part, but wrongly sets the high part to a sign extension of
itself rather than of the low part. This results in TCG internal
errors from the use of the uninitialized high part (in some GCC tests
of AArch64 NEON shift intrinsics, in particular). This patch corrects
the sign-extension logic, making it match other functions such as
tcg_gen_ld16s_i64.
Reviewed-by: Peter Maydell
Signed-off-by: Joseph Myers
Message-Id:
Signed-off-by: Richard Henderson
---
tcg/tcg-op.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tcg/tcg-op.c b/tcg/tcg-op.c
index 4d125df..6e2fb35 100644
--- a/tcg/tcg-op.c
+++ b/tcg/tcg-op.c
@@ -817,7 +817,7 @@ void tcg_gen_ld8u_i64(TCGv_i64 ret, TCGv_ptr arg2,
tcg_target_long offset)
void tcg_gen_ld8s_i64(TCGv_i64 ret, TCGv_ptr arg2, tcg_target_long offset)
{
tcg_gen_ld8s_i32(TCGV_LOW(ret), arg2, offset);
-tcg_gen_sari_i32(TCGV_HIGH(ret), TCGV_HIGH(ret), 31);
+tcg_gen_sari_i32(TCGV_HIGH(ret), TCGV_LOW(ret), 31);
}
void tcg_gen_ld16u_i64(TCGv_i64 ret, TCGv_ptr arg2, tcg_target_long offset)
--
2.7.4
[Qemu-devel] [PULL v2 for-2.8 4/9] log: Add locking to large logging blocks
Reuse the existing locking provided by stdio to keep in_asm, cpu,
op, op_opt, op_ind, and out_asm as contiguous blocks.
While it isn't possible to interleave e.g. in_asm or op_opt logs
because of the TB lock protecting all code generation, it is
possible to interleave cpu logs, or to interleave a cpu dump with
an out_asm dump.
Reviewed-by: Paolo Bonzini
Signed-off-by: Richard Henderson
---
cpu-exec.c| 2 ++
exec.c| 2 ++
include/qemu/log.h| 16
include/sysemu/os-posix.h | 12
include/sysemu/os-win32.h | 11 +++
target-alpha/translate.c | 2 ++
target-arm/translate-a64.c| 2 ++
target-arm/translate.c| 2 ++
target-cris/translate.c | 2 ++
target-i386/translate.c | 4
target-lm32/translate.c | 2 ++
target-m68k/translate.c | 2 ++
target-microblaze/translate.c | 2 ++
target-mips/translate.c | 2 ++
target-openrisc/translate.c | 2 ++
target-ppc/translate.c| 2 ++
target-s390x/translate.c | 2 ++
target-sh4/translate.c| 2 ++
target-sparc/translate.c | 2 ++
target-tilegx/translate.c | 6 +-
target-tricore/translate.c| 2 ++
target-unicore32/translate.c | 2 ++
target-xtensa/translate.c | 2 ++
tcg/tcg.c | 8
translate-all.c | 2 ++
25 files changed, 94 insertions(+), 1 deletion(-)
diff --git a/cpu-exec.c b/cpu-exec.c
index 3e40886..4188fed 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -150,11 +150,13 @@ static inline tcg_target_ulong cpu_tb_exec(CPUState *cpu,
TranslationBlock *itb)
#if defined(DEBUG_DISAS)
if (qemu_loglevel_mask(CPU_LOG_TB_CPU)
&& qemu_log_in_addr_range(itb->pc)) {
+qemu_log_lock();
#if defined(TARGET_I386)
log_cpu_state(cpu, CPU_DUMP_CCOP);
#else
log_cpu_state(cpu, 0);
#endif
+qemu_log_unlock();
}
#endif /* DEBUG_DISAS */
diff --git a/exec.c b/exec.c
index 4d08581..b1094c0 100644
--- a/exec.c
+++ b/exec.c
@@ -911,11 +911,13 @@ void cpu_abort(CPUState *cpu, const char *fmt, ...)
fprintf(stderr, "\n");
cpu_dump_state(cpu, stderr, fprintf, CPU_DUMP_FPU | CPU_DUMP_CCOP);
if (qemu_log_separate()) {
+qemu_log_lock();
qemu_log("qemu: fatal: ");
qemu_log_vprintf(fmt, ap2);
qemu_log("\n");
log_cpu_state(cpu, CPU_DUMP_FPU | CPU_DUMP_CCOP);
qemu_log_flush();
+qemu_log_unlock();
qemu_log_close();
}
va_end(ap2);
diff --git a/include/qemu/log.h b/include/qemu/log.h
index 00bf37f..a50e994 100644
--- a/include/qemu/log.h
+++ b/include/qemu/log.h
@@ -51,6 +51,22 @@ static inline bool qemu_loglevel_mask(int mask)
return (qemu_loglevel & mask) != 0;
}
+/* Lock output for a series of related logs. Since this is not needed
+ * for a single qemu_log / qemu_log_mask / qemu_log_mask_and_addr, we
+ * assume that qemu_loglevel_mask has already been tested, and that
+ * qemu_loglevel is never set when qemu_logfile is unset.
+ */
+
+static inline void qemu_log_lock(void)
+{
+qemu_flockfile(qemu_logfile);
+}
+
+static inline void qemu_log_unlock(void)
+{
+qemu_funlockfile(qemu_logfile);
+}
+
/* Logging functions: */
/* main logging function
diff --git a/include/sysemu/os-posix.h b/include/sysemu/os-posix.h
index 3cfedbc..b0a6c06 100644
--- a/include/sysemu/os-posix.h
+++ b/include/sysemu/os-posix.h
@@ -87,4 +87,16 @@ void *qemu_alloc_stack(size_t *sz);
*/
void qemu_free_stack(void *stack, size_t sz);
+/* POSIX and Mingw32 differ in the name of the stdio lock functions. */
+
+static inline void qemu_flockfile(FILE *f)
+{
+flockfile(f);
+}
+
+static inline void qemu_funlockfile(FILE *f)
+{
+funlockfile(f);
+}
+
#endif
diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h
index 17aad3b..7492004 100644
--- a/include/sysemu/os-win32.h
+++ b/include/sysemu/os-win32.h
@@ -103,6 +103,17 @@ static inline char *realpath(const char *path, char
*resolved_path)
return resolved_path;
}
+/* POSIX and Mingw32 differ in the name of the stdio lock functions. */
+
+static inline void qemu_flockfile(FILE *f)
+{
+_lock_file(f);
+}
+
+static inline void qemu_funlockfile(FILE *f)
+{
+_unlock_file(f);
+}
/* We wrap all the sockets functions so that we can
* set errno based on WSAGetLastError()
diff --git a/target-alpha/translate.c b/target-alpha/translate.c
index 03e4776..114927b 100644
--- a/target-alpha/translate.c
+++ b/target-alpha/translate.c
@@ -2994,9 +2994,11 @@ void gen_intermediate_code(CPUAlphaState *env, struct
TranslationBlock *tb)
#ifdef DEBUG_DISAS
if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)
&& qemu_log_in_addr_range(pc_start)) {
+qemu_log_lock();
qemu_log("IN: %s\n", lookup_symbol(pc_start));
log_target_disas(cs, pc_start, ctx.pc - pc_start, 1);
qemu_log("\n");
+qemu_log_unlock();
[Qemu-devel] [PULL v2 for-2.8 8/9] tcg/tcg.h: Improve documentation of TCGv_i32 etc types
From: Peter Maydell The typedefs we use for the TCGv_i32, TCGv_i64 and TCGv_ptr types are somewhat confusing, because we define them as pointers to structs, but the structs themselves are never defined. Explain in the comments a bit more clearly why this is OK and what is going on under the hood. Signed-off-by: Peter Maydell Message-Id: <1477067922-26202-1-git-send-email-peter.mayd...@linaro.org> Signed-off-by: Richard Henderson --- tcg/tcg.h | 38 ++ 1 file changed, 30 insertions(+), 8 deletions(-) diff --git a/tcg/tcg.h b/tcg/tcg.h index dc1281f..a35e4c4 100644 --- a/tcg/tcg.h +++ b/tcg/tcg.h @@ -376,14 +376,36 @@ static inline unsigned get_alignment_bits(TCGMemOp memop) typedef tcg_target_ulong TCGArg; -/* Define a type and accessor macros for variables. Using pointer types - is nice because it gives some level of type safely. Converting to and - from intptr_t rather than int reduces the number of sign-extension - instructions that get implied on 64-bit hosts. Users of tcg_gen_* don't - need to know about any of this, and should treat TCGv as an opaque type. - In addition we do typechecking for different types of variables. TCGv_i32 - and TCGv_i64 are 32/64-bit variables respectively. TCGv and TCGv_ptr - are aliases for target_ulong and host pointer sized values respectively. */ +/* Define type and accessor macros for TCG variables. + + TCG variables are the inputs and outputs of TCG ops, as described + in tcg/README. Target CPU front-end code uses these types to deal + with TCG variables as it emits TCG code via the tcg_gen_* functions. + They come in several flavours: +* TCGv_i32 : 32 bit integer type +* TCGv_i64 : 64 bit integer type +* TCGv_ptr : a host pointer type +* TCGv : an integer type the same size as target_ulong + (an alias for either TCGv_i32 or TCGv_i64) + The compiler's type checking will complain if you mix them + up and pass the wrong sized TCGv to a function. + + Users of tcg_gen_* don't need to know about any of the internal + details of these, and should treat them as opaque types. + You won't be able to look inside them in a debugger either. + + Internal implementation details follow: + + Note that there is no definition of the structs TCGv_i32_d etc anywhere. + This is deliberate, because the values we store in variables of type + TCGv_i32 are not really pointers-to-structures. They're just small + integers, but keeping them in pointer types like this means that the + compiler will complain if you accidentally pass a TCGv_i32 to a + function which takes a TCGv_i64, and so on. Only the internals of + TCG need to care about the actual contents of the types, and they always + box and unbox via the MAKE_TCGV_* and GET_TCGV_* functions. + Converting to and from intptr_t rather than int reduces the number + of sign-extension instructions that get implied on 64-bit hosts. */ typedef struct TCGv_i32_d *TCGv_i32; typedef struct TCGv_i64_d *TCGv_i64; -- 2.7.4
Re: [Qemu-devel] [PATCH v3 1/2] target-m68k: add 64bit mull
On 10/31/2016 05:48 PM, Laurent Vivier wrote: Signed-off-by: Laurent Vivier --- target-m68k/translate.c | 62 +++-- 1 file changed, 50 insertions(+), 12 deletions(-) Reviewed-by: Richard Henderson r~
[Qemu-devel] [PATCH v1 1/1] qemu-doc: update gluster protocol usage guide
Document:
1. The new debug and logfile options with their usages and
2. New json format and its usage.
Signed-off-by: Prasanna Kumar Kalever
---
qemu-doc.texi | 46 --
qemu-options.hx | 14 --
2 files changed, 44 insertions(+), 16 deletions(-)
diff --git a/qemu-doc.texi b/qemu-doc.texi
index 023c140..a7c5722 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -1041,35 +1041,50 @@ GlusterFS is an user space distributed file system.
You can boot from the GlusterFS disk image with the command:
@example
-qemu-system-x86_64 -drive
file=gluster[+@var{transport}]://[@var{server}[:@var{port}]]/@var{volname}/@var{image}[?socket=...]
+URI:
+qemu-system-x86_64 -drive
file=gluster[+@var{type}]://[@var{host}[:@var{port}]]/@var{volume}/@var{path}[?socket=...]
+
+JSON:
+qemu-system-x86_64
'json:@{"driver":"qcow2","file":@{"driver":"gluster","volume":"testvol","path":"a.img","debug":"N","logfile":"...","server":[@{"type":"tcp","host":"...","port":"..."@},@{"type":"unix","socket":"..."@}]@}@}'
@end example
@var{gluster} is the protocol.
-@var{transport} specifies the transport type used to connect to gluster
+@var{type} specifies the transport type used to connect to gluster
management daemon (glusterd). Valid transport types are
-tcp, unix and rdma. If a transport type isn't specified, then tcp
-type is assumed.
+tcp, unix. Incase of URI, if a transport type isn't specified,
+then tcp type is assumed.
-@var{server} specifies the server where the volume file specification for
-the given volume resides. This can be either hostname, ipv4 address
-or ipv6 address. ipv6 address needs to be within square brackets [ ].
-If transport type is unix, then @var{server} field should not be specified.
+@var{host} specifies the server where the volume file specification for
+the given volume resides. This can be either hostname, ipv4 address.
+If transport type is unix, then @var{host} field should not be specified.
Instead @var{socket} field needs to be populated with the path to unix domain
socket.
@var{port} is the port number on which glusterd is listening. This is optional
-and if not specified, QEMU will send 0 which will make gluster to use the
-default port. If the transport type is unix, then @var{port} should not be
-specified.
+and if not specified, it default to port 24007. If the transport type is unix,
+then @var{port} should not be specified.
+
+@var{volume} is the name of the gluster volume which contains the disk image.
+
+@var{path} is the path to the actual disk image that resides on gluster volume.
+
+@var{debug} is the logging level of the gluster protocol driver. Debug levels
+are 0-9, with 9 being the most verbose, and 0 representing no debugging output.
+Default is level of 4. The current logging levels defined in the gluster
source
+are 0 - None, 1 - Emergency, 2 - Alert, 3 - Critical, 4 - Error, 5 - Warning,
+6 - Notice, 7 - Info, 8 - Debug, 9 - Trace
+
+@var{logfile} is a commandline option to mention log file path which helps in
+logging to the specified file and also help in persisting the gfapi logs. The
+default is stderr.
+
-@var{volname} is the name of the gluster volume which contains the disk image.
-@var{image} is the path to the actual disk image that resides on gluster
volume.
You can create a GlusterFS disk image with the command:
@example
-qemu-img create gluster://@var{server}/@var{volname}/@var{image} @var{size}
+qemu-img create gluster://@var{host}/@var{volume}/@var{path} @var{size}
@end example
Examples
@@ -1082,6 +1097,9 @@ qemu-system-x86_64 -drive
file=gluster+tcp://[1:2:3:4:5:6:7:8]:24007/testvol/dir
qemu-system-x86_64 -drive
file=gluster+tcp://server.domain.com:24007/testvol/dir/a.img
qemu-system-x86_64 -drive
file=gluster+unix:///testvol/dir/a.img?socket=/tmp/glusterd.socket
qemu-system-x86_64 -drive file=gluster+rdma://1.2.3.4:24007/testvol/a.img
+qemu-system-x86_64 -drive
file=gluster://1.2.3.4/testvol/a.img,file.debug=9,file.logfile=/var/log/qemu-gluster.log
+qemu-system-x86_64
'json:@{"driver":"qcow2","file":@{"driver":"gluster","volume":"testvol","path":"a.img","debug":"9","logfile":"/var/log/qemu-gluster.log","server":[@{"type":"tcp","host":"1.2.3.4","port":24007@},@{"type":"unix","socket":"/var/run/glusterd.socket"@}]@}@}'
+qemu-system-x86_64 -drive
driver=qcow2,file.driver=gluster,file.volume=testvol,file.path=/path/a.img,file.debug=9,file.logfile=/var/log/qemu-gluster.log,file.server.0.type=tcp,file.server.0.host=1.2.3.4,file.server.0.port=24007,file.server.1.type=unix,file.server.1.socket=/var/run/glusterd.socket
@end example
@node disk_images_ssh
diff --git a/qemu-options.hx b/qemu-options.hx
index b1fbdb0..f5e8ccc 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2595,13 +2595,23 @@ TCP, Unix Domain Sockets and RDMA transport protocols.
Syntax for specifying a VM disk image on GlusterFS volume is
@example
-gluster[+transport]://[server[:port]]/volname/image[?s
Re: [Qemu-devel] [PATCH v3 2/2] target-m68k: add 680x0 divu/divs variants
On 10/31/2016 05:48 PM, Laurent Vivier wrote:
+void HELPER(divull)(CPUM68KState *env, int numr, int regr, uint32_t den)
+{
+uint32_t num = env->dregs[numr];
uint64_t num = deposit64(env->dregs[numr], 32, 32, env->dregs[regr]);
Similarly in divsll with int64_t num.
+if (!((quot >> 31) == 0 || (quot >> 31) == -1)) {
Maybe better as
if (quot != (int32_t)quot)
as in divsw.
Otherwise it looks good.
r~
[Qemu-devel] [PATCH] virtio-gpu: fix information leak in capset get dispatch
From: Li Qiang In virgl_cmd_get_capset function, it uses g_malloc to allocate a response struct to the guest. As the 'resp'struct hasn't been full initialized it will lead the 'resp->padding' field to the guest. Use g_malloc0 to avoid this. Signed-off-by: Li Qiang --- hw/display/virtio-gpu-3d.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/display/virtio-gpu-3d.c b/hw/display/virtio-gpu-3d.c index 23f39de..d98b140 100644 --- a/hw/display/virtio-gpu-3d.c +++ b/hw/display/virtio-gpu-3d.c @@ -371,7 +371,7 @@ static void virgl_cmd_get_capset(VirtIOGPU *g, virgl_renderer_get_cap_set(gc.capset_id, &max_ver, &max_size); -resp = g_malloc(sizeof(*resp) + max_size); +resp = g_malloc0(sizeof(*resp) + max_size); resp->hdr.type = VIRTIO_GPU_RESP_OK_CAPSET; virgl_renderer_fill_caps(gc.capset_id, -- 1.8.3.1
Re: [Qemu-devel] [PULL 00/47] virtio, pc: fixes and features
On 31 October 2016 at 22:48, Michael S. Tsirkin wrote: > On Mon, Oct 31, 2016 at 10:50:31AM +0100, Igor Mammedov wrote: >> On Sun, 30 Oct 2016 23:23:18 +0200 >> "Michael S. Tsirkin" wrote: >> >> > The following changes since commit >> > 5b2ecabaeabc17f032197246c4846b9ba95ba8a6: >> > >> > Merge remote-tracking branch 'remotes/kraxel/tags/pull-ui-20161028-1' >> > into staging (2016-10-28 17:59:04 +0100) >> > >> > are available in the git repository at: >> > >> > git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream >> > >> > for you to fetch changes up to f082ec0225bd15c71e0b4697d2df3af7bad65d7f: >> > >> > acpi: fix assert failure caused by commit 35c5a52d (2016-10-30 20:06:25 >> > +0200) >> > >> > >> > virtio, pc: fixes and features >> > >> > nvdimm hotplug support >> Michael, >> >> Could you drop nvdimm hotplug from pull request (I should review at least >> once as it touches not only NVDIMMs but a generic hotplug infrastructure) >> >> and keep only nvdimm fixes/cleanups for now? > > If I drop it now it won't be in the next QEMU and it seems like > a valuable feature. The comments so far are about minor style > improvements that IMO can be addressed by patches on top. > > We can always revert if you see bigger issues, but let's enable the > testing of this feature. I'm waiting for a reply from Igor before I do anything with this pull request. thanks -- PMM
Re: [Qemu-devel] [PULL for-2.8 0/5] qemu-ga patch queue for 2.8
On 1 November 2016 at 00:59, Michael Roth wrote: > The following changes since commit e80b4b8fb6babce7dcc91ea9ddeecbc351fd4646: > > Merge remote-tracking branch > 'remotes/awilliam/tags/vfio-updates-20161031.0' into staging (2016-10-31 > 18:19:06 +) > > are available in the git repository at: > > > git://github.com/mdroth/qemu.git tags/qga-pull-2016-10-31-tag > > for you to fetch changes up to 586ef5dee77180fc32e33bc08051600030630239: > > qga: add vsock-listen method (2016-10-31 19:49:33 -0500) > > > qemu-ga patch queue for 2.8 > > * add guest-fstrim support for w32 > * add support for using virtio-vsock as the communication channel > Applied, thanks. -- PMM
[Qemu-devel] [PULL v2 02/14] rbd: make the code more readable
From: Xiubo Li
Make it a bit clearer and more readable.
Signed-off-by: Xiubo Li
Reviewed-by: Stefan Hajnoczi
Reviewed-by: John Snow
Reviewed-by: Jeff Cody
Message-id: 1476519973-6436-1-git-send-email-lixi...@cmss.chinamobile.com
CC: John Snow
Signed-off-by: Jeff Cody
---
block/rbd.c | 29 ++---
1 file changed, 14 insertions(+), 15 deletions(-)
diff --git a/block/rbd.c b/block/rbd.c
index f6e1d4b..a57b3e3 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -365,45 +365,44 @@ static int qemu_rbd_create(const char *filename, QemuOpts
*opts, Error **errp)
rados_conf_read_file(cluster, NULL);
} else if (conf[0] != '\0' &&
qemu_rbd_set_conf(cluster, conf, true, &local_err) < 0) {
-rados_shutdown(cluster);
error_propagate(errp, local_err);
-return -EIO;
+ret = -EIO;
+goto shutdown;
}
if (conf[0] != '\0' &&
qemu_rbd_set_conf(cluster, conf, false, &local_err) < 0) {
-rados_shutdown(cluster);
error_propagate(errp, local_err);
-return -EIO;
+ret = -EIO;
+goto shutdown;
}
if (qemu_rbd_set_auth(cluster, secretid, errp) < 0) {
-rados_shutdown(cluster);
-return -EIO;
+ret = -EIO;
+goto shutdown;
}
ret = rados_connect(cluster);
if (ret < 0) {
error_setg_errno(errp, -ret, "error connecting");
-rados_shutdown(cluster);
-return ret;
+goto shutdown;
}
ret = rados_ioctx_create(cluster, pool, &io_ctx);
if (ret < 0) {
error_setg_errno(errp, -ret, "error opening pool %s", pool);
-rados_shutdown(cluster);
-return ret;
+goto shutdown;
}
ret = rbd_create(io_ctx, name, bytes, &obj_order);
+if (ret < 0) {
+error_setg_errno(errp, -ret, "error rbd create");
+}
+
rados_ioctx_destroy(io_ctx);
+
+shutdown:
rados_shutdown(cluster);
-if (ret < 0) {
-error_setg_errno(errp, -ret, "error rbd create");
-return ret;
-}
-
return ret;
}
--
2.7.4
[Qemu-devel] [PULL v2 00/14] Block patches for 2.8
The following changes since commit bf99fd3983d7185178a0f65ce29bb94b1aecaed1: Merge remote-tracking branch 'remotes/rth/tags/pull-sparc-20161031-2' into staging (2016-11-01 11:21:02 +) are available in the git repository at: g...@github.com:codyprime/qemu-kvm-jtc.git tags/block-pull-request for you to fetch changes up to d8996368106fbf133a6e52561a34f6d0f5080446: blockjobs: fix documentation (2016-11-01 08:04:56 -0400) Block patches for 2.8 Fam Zheng (1): block: Turn on "unmap" in active commit Jeff Cody (2): qapi: add release designator to gluster logfile option block: add gluster ifdef guard checks for SEEK_DATA/SEEK_HOLE support John Snow (7): blockjobs: hide internal jobs from management API blockjobs: Allow creating internal jobs Replication/Blockjobs: Create replication jobs as internal blockjob: centralize QMP event emissions Blockjobs: Internalize user_pause logic blockjobs: split interface into public/private, Part 1 blockjobs: fix documentation Prasanna Kumar Kalever (3): block/gluster: memory usage: use one glfs instance per volume block/gluster: improve defense over string to int conversion block/gluster: fix port type in the QAPI options list Xiubo Li (1): rbd: make the code more readable block/backup.c | 5 +- block/commit.c | 10 +- block/gluster.c | 124 block/mirror.c | 30 +++-- block/rbd.c | 29 +++-- block/replication.c | 14 +-- block/stream.c | 9 +- block/trace-events | 5 +- blockdev.c | 74 +--- blockjob.c | 113 ++ include/block/block.h| 3 +- include/block/block_int.h| 26 ++--- include/block/blockjob.h | 264 +++ include/block/blockjob_int.h | 239 +++ qapi/block-core.json | 2 +- qemu-img.c | 5 +- tests/test-blockjob-txn.c| 5 +- tests/test-blockjob.c| 4 +- 18 files changed, 574 insertions(+), 387 deletions(-) create mode 100644 include/block/blockjob_int.h -- 2.7.4
[Qemu-devel] [PULL v2 04/14] block/gluster: memory usage: use one glfs instance per volume
From: Prasanna Kumar Kalever
Currently, for every drive accessed via gfapi we create a new glfs
instance (call glfs_new() followed by glfs_init()) which could consume
memory in few 100 MB's, from the table below it looks like for each
instance ~300 MB VSZ was consumed
Before:
---
Disks VSZ RSS
1 1098728 187756
2 1430808 198656
3 1764932 199704
4 2084728 202684
This patch maintains a list of pre-opened glfs objects. On adding
a new drive belonging to the same gluster volume, we just reuse the
existing glfs object by updating its refcount.
With this approch we shrink up the unwanted memory consumption and
glfs_new/glfs_init calls for accessing a disk (file) if belongs to
same volume.
>From below table notice that the memory usage after adding a disk
(which will reuse the existing glfs object hence) is in negligible
compared to before.
After:
--
Disks VSZ RSS
1 1101964 185768
2 1109604 194920
3 1114012 196036
4 1114496 199868
Disks: number of -drive
VSZ: virtual memory size of the process in KiB
RSS: resident set size, the non-swapped physical memory (in kiloBytes)
VSZ and RSS are analyzed using 'ps aux' utility.
Signed-off-by: Prasanna Kumar Kalever
Reviewed-by: Jeff Cody
Message-id: 1477581890-4811-1-git-send-email-prasanna.kale...@redhat.com
Signed-off-by: Jeff Cody
---
block/gluster.c | 94 -
1 file changed, 80 insertions(+), 14 deletions(-)
diff --git a/block/gluster.c b/block/gluster.c
index 1735d12..40bd29c 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -56,6 +56,19 @@ typedef struct BDRVGlusterReopenState {
} BDRVGlusterReopenState;
+typedef struct GlfsPreopened {
+char *volume;
+glfs_t *fs;
+int ref;
+} GlfsPreopened;
+
+typedef struct ListElement {
+QLIST_ENTRY(ListElement) list;
+GlfsPreopened saved;
+} ListElement;
+
+static QLIST_HEAD(glfs_list, ListElement) glfs_list;
+
static QemuOptsList qemu_gluster_create_opts = {
.name = "qemu-gluster-create-opts",
.head = QTAILQ_HEAD_INITIALIZER(qemu_gluster_create_opts.head),
@@ -194,6 +207,57 @@ static QemuOptsList runtime_tcp_opts = {
},
};
+static void glfs_set_preopened(const char *volume, glfs_t *fs)
+{
+ListElement *entry = NULL;
+
+entry = g_new(ListElement, 1);
+
+entry->saved.volume = g_strdup(volume);
+
+entry->saved.fs = fs;
+entry->saved.ref = 1;
+
+QLIST_INSERT_HEAD(&glfs_list, entry, list);
+}
+
+static glfs_t *glfs_find_preopened(const char *volume)
+{
+ListElement *entry = NULL;
+
+ QLIST_FOREACH(entry, &glfs_list, list) {
+if (strcmp(entry->saved.volume, volume) == 0) {
+entry->saved.ref++;
+return entry->saved.fs;
+}
+ }
+
+return NULL;
+}
+
+static void glfs_clear_preopened(glfs_t *fs)
+{
+ListElement *entry = NULL;
+
+if (fs == NULL) {
+return;
+}
+
+QLIST_FOREACH(entry, &glfs_list, list) {
+if (entry->saved.fs == fs) {
+if (--entry->saved.ref) {
+return;
+}
+
+QLIST_REMOVE(entry, list);
+
+glfs_fini(entry->saved.fs);
+g_free(entry->saved.volume);
+g_free(entry);
+}
+}
+}
+
static int parse_volume_options(BlockdevOptionsGluster *gconf, char *path)
{
char *p, *q;
@@ -331,11 +395,18 @@ static struct glfs
*qemu_gluster_glfs_init(BlockdevOptionsGluster *gconf,
int old_errno;
GlusterServerList *server;
+glfs = glfs_find_preopened(gconf->volume);
+if (glfs) {
+return glfs;
+}
+
glfs = glfs_new(gconf->volume);
if (!glfs) {
goto out;
}
+glfs_set_preopened(gconf->volume, glfs);
+
for (server = gconf->server; server; server = server->next) {
if (server->value->type == GLUSTER_TRANSPORT_UNIX) {
ret = glfs_set_volfile_server(glfs,
@@ -387,7 +458,7 @@ static struct glfs
*qemu_gluster_glfs_init(BlockdevOptionsGluster *gconf,
out:
if (glfs) {
old_errno = errno;
-glfs_fini(glfs);
+glfs_clear_preopened(glfs);
errno = old_errno;
}
return NULL;
@@ -767,9 +838,9 @@ out:
if (s->fd) {
glfs_close(s->fd);
}
-if (s->glfs) {
-glfs_fini(s->glfs);
-}
+
+glfs_clear_preopened(s->glfs);
+
return ret;
}
@@ -836,9 +907,8 @@ static void qemu_gluster_reopen_commit(BDRVReopenState
*state)
if (s->fd) {
glfs_close(s->fd);
}
-if (s->glfs) {
-glfs_fini(s->glfs);
-}
+
+glfs_clear_preopened(s->glfs);
/* use the newly opened image / connection */
s->fd = reop_s->fd;
@@ -863,9 +933,7 @@ static void qemu_gluster_reopen_abort(BDRVReopenState
*state)
glfs_close(reop_s->fd);
}
-if (reop_s->glfs) {
-glfs_fini(reop_s->glfs);
-}
+glfs_clear_preopened(reop_s->glfs);
g_free(state->opaque);
stat
[Qemu-devel] [PULL v2 01/14] qapi: add release designator to gluster logfile option
The "logfile" option to BlockdevOptionsGluster will not be in QEMU until 2.8. Update comment to indicate this. Reported-by: Eric Blake Signed-off-by: Jeff Cody --- qapi/block-core.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qapi/block-core.json b/qapi/block-core.json index 5af040b..bcd3b9e 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -2197,7 +2197,7 @@ # # @debug-level: #optional libgfapi log level (default '4' which is Error) # -# @logfile: #optional libgfapi log file (default /dev/stderr) +# @logfile: #optional libgfapi log file (default /dev/stderr) (Since 2.8) # # Since: 2.7 ## -- 2.7.4
[Qemu-devel] [PULL v2 03/14] block: add gluster ifdef guard checks for SEEK_DATA/SEEK_HOLE support
Add checks to see if the system compiling QEMU has support for
SEEK_HOLE/SEEK_DATA. If the system does not, we will flag that seek
data is unsupported in gluster.
Note: this is not a check on whether the gluster server itself supports
SEEK_DATA (that is already done during runtime), but rather if the
compilation environment supports SEEK_DATA.
Reviewed-by: Eric Blake
Signed-off-by: Jeff Cody
Tested-by: Eric Blake
Message-id:
00370bce5c98140d6c56ad5145635ec6551265cc.1475876377.git.jc...@redhat.com
Signed-off-by: Jeff Cody
---
block/gluster.c | 17 ++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/block/gluster.c b/block/gluster.c
index af76d7d..1735d12 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -668,7 +668,10 @@ static void qemu_gluster_parse_flags(int bdrv_flags, int
*open_flags)
*/
static bool qemu_gluster_test_seek(struct glfs_fd *fd)
{
-off_t ret, eof;
+off_t ret = 0;
+
+#if defined SEEK_HOLE && defined SEEK_DATA
+off_t eof;
eof = glfs_lseek(fd, 0, SEEK_END);
if (eof < 0) {
@@ -678,6 +681,8 @@ static bool qemu_gluster_test_seek(struct glfs_fd *fd)
/* this should always fail with ENXIO if SEEK_DATA is supported */
ret = glfs_lseek(fd, eof, SEEK_DATA);
+#endif
+
return (ret < 0) && (errno == ENXIO);
}
@@ -1178,12 +1183,14 @@ static int find_allocation(BlockDriverState *bs, off_t
start,
off_t *data, off_t *hole)
{
BDRVGlusterState *s = bs->opaque;
-off_t offs;
if (!s->supports_seek_data) {
-return -ENOTSUP;
+goto exit;
}
+#if defined SEEK_HOLE && defined SEEK_DATA
+off_t offs;
+
/*
* SEEK_DATA cases:
* D1. offs == start: start is in data
@@ -1247,6 +1254,10 @@ static int find_allocation(BlockDriverState *bs, off_t
start,
/* D1 and H1 */
return -EBUSY;
+#endif
+
+exit:
+return -ENOTSUP;
}
/*
--
2.7.4
[Qemu-devel] [PULL v2 08/14] blockjobs: hide internal jobs from management API
From: John Snow
If jobs are not created directly by the user, do not allow them to be
seen by the user/management utility. At the moment, 'internal' jobs are
those that do not have an ID. As of this patch it is impossible to
create such jobs.
Signed-off-by: John Snow
Message-id: 1477584421-1399-2-git-send-email-js...@redhat.com
Signed-off-by: Jeff Cody
---
blockdev.c | 17 +
blockjob.c | 41 ++---
include/block/blockjob.h | 12 ++--
3 files changed, 57 insertions(+), 13 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index ded1326..d57cb0c 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3946,13 +3946,22 @@ BlockJobInfoList *qmp_query_block_jobs(Error **errp)
BlockJob *job;
for (job = block_job_next(NULL); job; job = block_job_next(job)) {
-BlockJobInfoList *elem = g_new0(BlockJobInfoList, 1);
-AioContext *aio_context = blk_get_aio_context(job->blk);
+BlockJobInfoList *elem;
+AioContext *aio_context;
+if (block_job_is_internal(job)) {
+continue;
+}
+elem = g_new0(BlockJobInfoList, 1);
+aio_context = blk_get_aio_context(job->blk);
aio_context_acquire(aio_context);
-elem->value = block_job_query(job);
+elem->value = block_job_query(job, errp);
aio_context_release(aio_context);
-
+if (!elem->value) {
+g_free(elem);
+qapi_free_BlockJobInfoList(head);
+return NULL;
+}
*p_next = elem;
p_next = &elem->next;
}
diff --git a/blockjob.c b/blockjob.c
index 422851f..84d4f75 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -66,7 +66,7 @@ BlockJob *block_job_get(const char *id)
BlockJob *job;
QLIST_FOREACH(job, &block_jobs, job_list) {
-if (!strcmp(id, job->id)) {
+if (job->id && !strcmp(id, job->id)) {
return job;
}
}
@@ -188,6 +188,11 @@ void *block_job_create(const char *job_id, const
BlockJobDriver *driver,
return job;
}
+bool block_job_is_internal(BlockJob *job)
+{
+return (job->id == NULL);
+}
+
void block_job_ref(BlockJob *job)
{
++job->refcnt;
@@ -330,6 +335,8 @@ void block_job_set_speed(BlockJob *job, int64_t speed,
Error **errp)
void block_job_complete(BlockJob *job, Error **errp)
{
+/* Should not be reachable via external interface for internal jobs */
+assert(job->id);
if (job->pause_count || job->cancelled || !job->driver->complete) {
error_setg(errp, "The active block job '%s' cannot be completed",
job->id);
@@ -510,9 +517,15 @@ void block_job_yield(BlockJob *job)
block_job_pause_point(job);
}
-BlockJobInfo *block_job_query(BlockJob *job)
+BlockJobInfo *block_job_query(BlockJob *job, Error **errp)
{
-BlockJobInfo *info = g_new0(BlockJobInfo, 1);
+BlockJobInfo *info;
+
+if (block_job_is_internal(job)) {
+error_setg(errp, "Cannot query QEMU internal jobs");
+return NULL;
+}
+info = g_new0(BlockJobInfo, 1);
info->type = g_strdup(BlockJobType_lookup[job->driver->job_type]);
info->device= g_strdup(job->id);
info->len = job->len;
@@ -535,6 +548,10 @@ static void block_job_iostatus_set_err(BlockJob *job, int
error)
void block_job_event_cancelled(BlockJob *job)
{
+if (block_job_is_internal(job)) {
+return;
+}
+
qapi_event_send_block_job_cancelled(job->driver->job_type,
job->id,
job->len,
@@ -545,6 +562,10 @@ void block_job_event_cancelled(BlockJob *job)
void block_job_event_completed(BlockJob *job, const char *msg)
{
+if (block_job_is_internal(job)) {
+return;
+}
+
qapi_event_send_block_job_completed(job->driver->job_type,
job->id,
job->len,
@@ -559,6 +580,10 @@ void block_job_event_ready(BlockJob *job)
{
job->ready = true;
+if (block_job_is_internal(job)) {
+return;
+}
+
qapi_event_send_block_job_ready(job->driver->job_type,
job->id,
job->len,
@@ -589,10 +614,12 @@ BlockErrorAction block_job_error_action(BlockJob *job,
BlockdevOnError on_err,
default:
abort();
}
-qapi_event_send_block_job_error(job->id,
-is_read ? IO_OPERATION_TYPE_READ :
-IO_OPERATION_TYPE_WRITE,
-action, &error_abort);
+if (!block_job_is_internal(job)) {
+qapi_event_send_block_job_error(job->id,
+is_read ? IO_OPERATION_TYPE_READ :
+IO_OPERATION_TYPE_WRITE,
+action, &error_abort);
+}
if (acti
[Qemu-devel] [PULL v2 13/14] blockjobs: split interface into public/private, Part 1
From: John Snow
To make it a little more obvious which functions are intended to be
public interface and which are intended to be for use only by jobs
themselves, split the interface into "public" and "private" files.
Convert blockjobs (e.g. block/backup) to using the private interface.
Leave blockdev and others on the public interface.
There are remaining uses of private state by qemu-img, and several
cases in blockdev.c and block/io.c where we grab job->blk for the
purposes of acquiring an AIOContext.
These will be corrected in future patches.
Signed-off-by: John Snow
Reviewed-by: Kevin Wolf
Reviewed-by: Jeff Cody
Message-id: 1477584421-1399-7-git-send-email-js...@redhat.com
Signed-off-by: Jeff Cody
---
block/backup.c | 2 +-
block/commit.c | 2 +-
block/mirror.c | 2 +-
block/stream.c | 2 +-
blockjob.c | 2 +-
include/block/block.h| 3 +-
include/block/blockjob.h | 212 +-
include/block/blockjob_int.h | 239 +++
tests/test-blockjob-txn.c| 2 +-
tests/test-blockjob.c| 2 +-
10 files changed, 251 insertions(+), 217 deletions(-)
create mode 100644 include/block/blockjob_int.h
diff --git a/block/backup.c b/block/backup.c
index 2a369e6..7b5d8a3 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -16,7 +16,7 @@
#include "trace.h"
#include "block/block.h"
#include "block/block_int.h"
-#include "block/blockjob.h"
+#include "block/blockjob_int.h"
#include "block/block_backup.h"
#include "qapi/error.h"
#include "qapi/qmp/qerror.h"
diff --git a/block/commit.c b/block/commit.c
index 18ec578..e1eda89 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -15,7 +15,7 @@
#include "qemu/osdep.h"
#include "trace.h"
#include "block/block_int.h"
-#include "block/blockjob.h"
+#include "block/blockjob_int.h"
#include "qapi/error.h"
#include "qapi/qmp/qerror.h"
#include "qemu/ratelimit.h"
diff --git a/block/mirror.c b/block/mirror.c
index aa60bcc..b2c1fb8 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -13,7 +13,7 @@
#include "qemu/osdep.h"
#include "trace.h"
-#include "block/blockjob.h"
+#include "block/blockjob_int.h"
#include "block/block_int.h"
#include "sysemu/block-backend.h"
#include "qapi/error.h"
diff --git a/block/stream.c b/block/stream.c
index 152c1be..b05856b 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -14,7 +14,7 @@
#include "qemu/osdep.h"
#include "trace.h"
#include "block/block_int.h"
-#include "block/blockjob.h"
+#include "block/blockjob_int.h"
#include "qapi/error.h"
#include "qapi/qmp/qerror.h"
#include "qemu/ratelimit.h"
diff --git a/blockjob.c b/blockjob.c
index d880ad2..4aa14a4 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -27,7 +27,7 @@
#include "qemu-common.h"
#include "trace.h"
#include "block/block.h"
-#include "block/blockjob.h"
+#include "block/blockjob_int.h"
#include "block/block_int.h"
#include "sysemu/block-backend.h"
#include "qapi/qmp/qerror.h"
diff --git a/include/block/block.h b/include/block/block.h
index b81a3e3..49bb0b2 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -7,16 +7,15 @@
#include "qemu/coroutine.h"
#include "block/accounting.h"
#include "block/dirty-bitmap.h"
+#include "block/blockjob.h"
#include "qapi/qmp/qobject.h"
#include "qapi-types.h"
#include "qemu/hbitmap.h"
/* block.c */
typedef struct BlockDriver BlockDriver;
-typedef struct BlockJob BlockJob;
typedef struct BdrvChild BdrvChild;
typedef struct BdrvChildRole BdrvChildRole;
-typedef struct BlockJobTxn BlockJobTxn;
typedef struct BlockDriverInfo {
/* in bytes, 0 if irrelevant */
diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index d31ea43..356cacf 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -28,85 +28,15 @@
#include "block/block.h"
-/**
- * BlockJobDriver:
- *
- * A class type for block job driver.
- */
-typedef struct BlockJobDriver {
-/** Derived BlockJob struct size */
-size_t instance_size;
-
-/** String describing the operation, part of query-block-jobs QMP API */
-BlockJobType job_type;
-
-/** Optional callback for job types that support setting a speed limit */
-void (*set_speed)(BlockJob *job, int64_t speed, Error **errp);
-
-/** Optional callback for job types that need to forward I/O status reset
*/
-void (*iostatus_reset)(BlockJob *job);
-
-/**
- * Optional callback for job types whose completion must be triggered
- * manually.
- */
-void (*complete)(BlockJob *job, Error **errp);
-
-/**
- * If the callback is not NULL, it will be invoked when all the jobs
- * belonging to the same transaction complete; or upon this job's
- * completion if it is not in a transaction. Skipped if NULL.
- *
- * All jobs will complete with a call to either .commit() or .abort() but
- * never both.
- */
-void (*commit)
[Qemu-devel] [PULL v2 06/14] block/gluster: improve defense over string to int conversion
From: Prasanna Kumar Kalever
using atoi() for converting string to int may be error prone in case if
string supplied in the argument is not a fold of numerical number,
This is not a bug because in the existing code,
static QemuOptsList runtime_tcp_opts = {
.name = "gluster_tcp",
.head = QTAILQ_HEAD_INITIALIZER(runtime_tcp_opts.head),
.desc = {
...
{
.name = GLUSTER_OPT_PORT,
.type = QEMU_OPT_NUMBER,
.help = "port number ...",
},
...
};
port type is QEMU_OPT_NUMBER, before we actually reaches atoi() port is already
defended by parse_option_number()
However It is a good practice to use function like parse_uint_full()
over atoi() to keep port self defended
Note: As now the port string to int conversion has its defence code set,
and also we understand that port argument is actually a string type,
in the follow up patch let's move port type from QEMU_OPT_NUMBER to
QEMU_OPT_STRING
[Jeff Cody: removed spurious parenthesis]
Signed-off-by: Prasanna Kumar Kalever
Reviewed-by: Markus Armbruster
Signed-off-by: Jeff Cody
---
block/gluster.c | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/block/gluster.c b/block/gluster.c
index 40bd29c..98a9132 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -14,6 +14,7 @@
#include "qapi/qmp/qerror.h"
#include "qemu/uri.h"
#include "qemu/error-report.h"
+#include "qemu/cutils.h"
#define GLUSTER_OPT_FILENAME"filename"
#define GLUSTER_OPT_VOLUME "volume"
@@ -394,6 +395,7 @@ static struct glfs
*qemu_gluster_glfs_init(BlockdevOptionsGluster *gconf,
int ret;
int old_errno;
GlusterServerList *server;
+unsigned long long port;
glfs = glfs_find_preopened(gconf->volume);
if (glfs) {
@@ -413,10 +415,17 @@ static struct glfs
*qemu_gluster_glfs_init(BlockdevOptionsGluster *gconf,
GlusterTransport_lookup[server->value->type],
server->value->u.q_unix.path, 0);
} else {
+if (parse_uint_full(server->value->u.tcp.port, &port, 10) < 0 ||
+port > 65535) {
+error_setg(errp, "'%s' is not a valid port number",
+ server->value->u.tcp.port);
+errno = EINVAL;
+goto out;
+}
ret = glfs_set_volfile_server(glfs,
GlusterTransport_lookup[server->value->type],
server->value->u.tcp.host,
- atoi(server->value->u.tcp.port));
+ (int)port);
}
if (ret < 0) {
--
2.7.4
[Qemu-devel] [PULL v2 05/14] block: Turn on "unmap" in active commit
From: Fam Zheng
We already specified BDRV_O_UNMAP when opening images in 'qemu-img
commit', but didn't turn on the "unmap" in the active commit job. This
patch fixes that so that zeroed clusters in top image can be discarded
which is desired in the virt-sparsify use case, where a temporary
overlay is created and fstrim'ed before commiting back, to free space in
the original image.
This also enables it for block-commit.
Signed-off-by: Fam Zheng
Reviewed-by: Eric Blake
Message-id: 1474974892-5031-1-git-send-email-f...@redhat.com
Signed-off-by: Jeff Cody
---
block/mirror.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/block/mirror.c b/block/mirror.c
index 7e99f3a..82a9529 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -1081,7 +1081,7 @@ void commit_active_start(const char *job_id,
BlockDriverState *bs,
mirror_start_job(job_id, bs, base, NULL, speed, 0, 0,
MIRROR_LEAVE_BACKING_CHAIN,
- on_error, on_error, false, cb, opaque, &local_err,
+ on_error, on_error, true, cb, opaque, &local_err,
&commit_active_job_driver, false, base, auto_complete);
if (local_err) {
error_propagate(errp, local_err);
--
2.7.4
[Qemu-devel] [PULL v2 12/14] Blockjobs: Internalize user_pause logic
From: John Snow
BlockJobs will begin hiding their state in preparation for some
refactorings anyway, so let's internalize the user_pause mechanism
instead of leaving it to callers to correctly manage.
Signed-off-by: John Snow
Reviewed-by: Kevin Wolf
Reviewed-by: Jeff Cody
Message-id: 1477584421-1399-6-git-send-email-js...@redhat.com
Signed-off-by: Jeff Cody
---
blockdev.c | 12 +---
blockjob.c | 22 --
include/block/blockjob.h | 26 ++
3 files changed, 51 insertions(+), 9 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index b6a74916..102ca9f 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3610,7 +3610,7 @@ void qmp_block_job_cancel(const char *device,
force = false;
}
-if (job->user_paused && !force) {
+if (block_job_user_paused(job) && !force) {
error_setg(errp, "The block job for device '%s' is currently paused",
device);
goto out;
@@ -3627,13 +3627,12 @@ void qmp_block_job_pause(const char *device, Error
**errp)
AioContext *aio_context;
BlockJob *job = find_block_job(device, &aio_context, errp);
-if (!job || job->user_paused) {
+if (!job || block_job_user_paused(job)) {
return;
}
-job->user_paused = true;
trace_qmp_block_job_pause(job);
-block_job_pause(job);
+block_job_user_pause(job);
aio_context_release(aio_context);
}
@@ -3642,14 +3641,13 @@ void qmp_block_job_resume(const char *device, Error
**errp)
AioContext *aio_context;
BlockJob *job = find_block_job(device, &aio_context, errp);
-if (!job || !job->user_paused) {
+if (!job || !block_job_user_paused(job)) {
return;
}
-job->user_paused = false;
trace_qmp_block_job_resume(job);
block_job_iostatus_reset(job);
-block_job_resume(job);
+block_job_user_resume(job);
aio_context_release(aio_context);
}
diff --git a/blockjob.c b/blockjob.c
index 309ef9a..d880ad2 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -373,11 +373,22 @@ void block_job_pause(BlockJob *job)
job->pause_count++;
}
+void block_job_user_pause(BlockJob *job)
+{
+job->user_paused = true;
+block_job_pause(job);
+}
+
static bool block_job_should_pause(BlockJob *job)
{
return job->pause_count > 0;
}
+bool block_job_user_paused(BlockJob *job)
+{
+return job ? job->user_paused : 0;
+}
+
void coroutine_fn block_job_pause_point(BlockJob *job)
{
if (!block_job_should_pause(job)) {
@@ -414,6 +425,14 @@ void block_job_resume(BlockJob *job)
block_job_enter(job);
}
+void block_job_user_resume(BlockJob *job)
+{
+if (job && job->user_paused && job->pause_count > 0) {
+job->user_paused = false;
+block_job_resume(job);
+}
+}
+
void block_job_enter(BlockJob *job)
{
if (job->co && !job->busy) {
@@ -644,8 +663,7 @@ BlockErrorAction block_job_error_action(BlockJob *job,
BlockdevOnError on_err,
}
if (action == BLOCK_ERROR_ACTION_STOP) {
/* make the pause user visible, which will be resumed from QMP. */
-job->user_paused = true;
-block_job_pause(job);
+block_job_user_pause(job);
block_job_iostatus_set_err(job, error);
}
return action;
diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index c031fe7..d31ea43 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -379,6 +379,23 @@ void coroutine_fn block_job_pause_point(BlockJob *job);
void block_job_pause(BlockJob *job);
/**
+ * block_job_user_pause:
+ * @job: The job to be paused.
+ *
+ * Asynchronously pause the specified job.
+ * Do not allow a resume until a matching call to block_job_user_resume.
+ */
+void block_job_user_pause(BlockJob *job);
+
+/**
+ * block_job_paused:
+ * @job: The job to query.
+ *
+ * Returns true if the job is user-paused.
+ */
+bool block_job_user_paused(BlockJob *job);
+
+/**
* block_job_resume:
* @job: The job to be resumed.
*
@@ -387,6 +404,15 @@ void block_job_pause(BlockJob *job);
void block_job_resume(BlockJob *job);
/**
+ * block_job_user_resume:
+ * @job: The job to be resumed.
+ *
+ * Resume the specified job.
+ * Must be paired with a preceding block_job_user_pause.
+ */
+void block_job_user_resume(BlockJob *job);
+
+/**
* block_job_enter:
* @job: The job to enter.
*
--
2.7.4
[Qemu-devel] [PULL v2 07/14] block/gluster: fix port type in the QAPI options list
From: Prasanna Kumar Kalever
After introduction of qapi schema in gluster block driver code, the port
type is now string as per InetSocketAddress
{ 'struct': 'InetSocketAddress',
'data': {
'host': 'str',
'port': 'str',
'*to': 'uint16',
'*ipv4': 'bool',
'*ipv6': 'bool' } }
but the current code still treats it as QEMU_OPT_NUMBER, hence fixing port
to accept QEMU_OPT_STRING.
Suggested-by: Markus Armbruster
Signed-off-by: Prasanna Kumar Kalever
Reviewed-by: Jeff Cody
Signed-off-by: Jeff Cody
---
block/gluster.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/block/gluster.c b/block/gluster.c
index 98a9132..0ce15f7 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -186,7 +186,7 @@ static QemuOptsList runtime_tcp_opts = {
},
{
.name = GLUSTER_OPT_PORT,
-.type = QEMU_OPT_NUMBER,
+.type = QEMU_OPT_STRING,
.help = "port number on which glusterd is listening (default
24007)",
},
{
--
2.7.4
Re: [Qemu-devel] [PATCH] virtio-gpu: fix information leak in capset get dispatch
Hi On Tue, Nov 1, 2016 at 3:38 PM Li Qiang wrote: > From: Li Qiang > > In virgl_cmd_get_capset function, it uses g_malloc to allocate > a response struct to the guest. As the 'resp'struct hasn't been full > initialized it will lead the 'resp->padding' field to the guest. > Use g_malloc0 to avoid this. > > Signed-off-by: Li Qiang > I was about to point out this in the previous memset patch Reviewed-by: Marc-André Lureau --- > hw/display/virtio-gpu-3d.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/display/virtio-gpu-3d.c b/hw/display/virtio-gpu-3d.c > index 23f39de..d98b140 100644 > --- a/hw/display/virtio-gpu-3d.c > +++ b/hw/display/virtio-gpu-3d.c > @@ -371,7 +371,7 @@ static void virgl_cmd_get_capset(VirtIOGPU *g, > > virgl_renderer_get_cap_set(gc.capset_id, &max_ver, > &max_size); > -resp = g_malloc(sizeof(*resp) + max_size); > +resp = g_malloc0(sizeof(*resp) + max_size); > > resp->hdr.type = VIRTIO_GPU_RESP_OK_CAPSET; > virgl_renderer_fill_caps(gc.capset_id, > -- > 1.8.3.1 > > > -- Marc-André Lureau
[Qemu-devel] [PULL v2 09/14] blockjobs: Allow creating internal jobs
From: John Snow
Add the ability to create jobs without an ID.
Signed-off-by: John Snow
Reviewed-by: Kevin Wolf
Reviewed-by: Jeff Cody
Message-id: 1477584421-1399-3-git-send-email-js...@redhat.com
Signed-off-by: Jeff Cody
---
block/backup.c| 2 +-
block/commit.c| 2 +-
block/mirror.c| 3 ++-
block/stream.c| 2 +-
blockjob.c| 25 -
include/block/blockjob.h | 7 ++-
tests/test-blockjob-txn.c | 3 ++-
tests/test-blockjob.c | 2 +-
8 files changed, 30 insertions(+), 16 deletions(-)
diff --git a/block/backup.c b/block/backup.c
index 44c7ff3..3877d93 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -612,7 +612,7 @@ void backup_start(const char *job_id, BlockDriverState *bs,
}
job = block_job_create(job_id, &backup_job_driver, bs, speed,
- cb, opaque, errp);
+ BLOCK_JOB_DEFAULT, cb, opaque, errp);
if (!job) {
goto error;
}
diff --git a/block/commit.c b/block/commit.c
index a5e17f6..0740a41 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -234,7 +234,7 @@ void commit_start(const char *job_id, BlockDriverState *bs,
}
s = block_job_create(job_id, &commit_job_driver, bs, speed,
- cb, opaque, errp);
+ BLOCK_JOB_DEFAULT, cb, opaque, errp);
if (!s) {
return;
}
diff --git a/block/mirror.c b/block/mirror.c
index 82a9529..e9fba9b 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -967,7 +967,8 @@ static void mirror_start_job(const char *job_id,
BlockDriverState *bs,
buf_size = DEFAULT_MIRROR_BUF_SIZE;
}
-s = block_job_create(job_id, driver, bs, speed, cb, opaque, errp);
+s = block_job_create(job_id, driver, bs, speed,
+ BLOCK_JOB_DEFAULT, cb, opaque, errp);
if (!s) {
return;
}
diff --git a/block/stream.c b/block/stream.c
index b8ab89a..09ce9ef 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -230,7 +230,7 @@ void stream_start(const char *job_id, BlockDriverState *bs,
int orig_bs_flags;
s = block_job_create(job_id, &stream_job_driver, bs, speed,
- cb, opaque, errp);
+ BLOCK_JOB_DEFAULT, cb, opaque, errp);
if (!s) {
return;
}
diff --git a/blockjob.c b/blockjob.c
index 84d4f75..c286fc3 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -121,7 +121,7 @@ void block_job_add_bdrv(BlockJob *job, BlockDriverState *bs)
}
void *block_job_create(const char *job_id, const BlockJobDriver *driver,
- BlockDriverState *bs, int64_t speed,
+ BlockDriverState *bs, int64_t speed, int flags,
BlockCompletionFunc *cb, void *opaque, Error **errp)
{
BlockBackend *blk;
@@ -133,7 +133,7 @@ void *block_job_create(const char *job_id, const
BlockJobDriver *driver,
return NULL;
}
-if (job_id == NULL) {
+if (job_id == NULL && !(flags & BLOCK_JOB_INTERNAL)) {
job_id = bdrv_get_device_name(bs);
if (!*job_id) {
error_setg(errp, "An explicit job ID is required for this node");
@@ -141,14 +141,21 @@ void *block_job_create(const char *job_id, const
BlockJobDriver *driver,
}
}
-if (!id_wellformed(job_id)) {
-error_setg(errp, "Invalid job ID '%s'", job_id);
-return NULL;
-}
+if (job_id) {
+if (flags & BLOCK_JOB_INTERNAL) {
+error_setg(errp, "Cannot specify job ID for internal block job");
+return NULL;
+}
-if (block_job_get(job_id)) {
-error_setg(errp, "Job ID '%s' already in use", job_id);
-return NULL;
+if (!id_wellformed(job_id)) {
+error_setg(errp, "Invalid job ID '%s'", job_id);
+return NULL;
+}
+
+if (block_job_get(job_id)) {
+error_setg(errp, "Job ID '%s' already in use", job_id);
+return NULL;
+}
}
blk = blk_new();
diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index a1b7502..d0d9333 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -210,6 +210,11 @@ struct BlockJob {
QLIST_ENTRY(BlockJob) txn_list;
};
+typedef enum BlockJobCreateFlags {
+BLOCK_JOB_DEFAULT = 0x00,
+BLOCK_JOB_INTERNAL = 0x01,
+} BlockJobCreateFlags;
+
/**
* block_job_next:
* @job: A block job, or %NULL.
@@ -252,7 +257,7 @@ BlockJob *block_job_get(const char *id);
* called from a wrapper that is specific to the job type.
*/
void *block_job_create(const char *job_id, const BlockJobDriver *driver,
- BlockDriverState *bs, int64_t speed,
+ BlockDriverState *bs, int64_t speed, int flags,
BlockCompletionFunc *cb, void *opaque, Error **errp);
/**
diff --git a/tests/test-blockjob-txn.c b/tests/test-blockjob-txn.c
index d0
[Qemu-devel] [PULL v2 14/14] blockjobs: fix documentation
From: John Snow (Trivial) Fix wrong function names in documentation. Signed-off-by: John Snow Reviewed-by: Kevin Wolf Reviewed-by: Jeff Cody Message-id: 1477584421-1399-8-git-send-email-js...@redhat.com Signed-off-by: Jeff Cody --- include/block/blockjob_int.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h index 2b1e859..40275e4 100644 --- a/include/block/blockjob_int.h +++ b/include/block/blockjob_int.h @@ -198,8 +198,8 @@ void coroutine_fn block_job_pause_point(BlockJob *job); void block_job_enter(BlockJob *job); /** - * block_job_ready: - * @job: The job which is now ready to complete. + * block_job_event_ready: + * @job: The job which is now ready to be completed. * * Send a BLOCK_JOB_READY event for the specified job. */ -- 2.7.4
[Qemu-devel] [PULL v2 10/14] Replication/Blockjobs: Create replication jobs as internal
From: John Snow
Bubble up the internal interface to commit and backup jobs, then switch
replication tasks over to using this methodology.
Signed-off-by: John Snow
Reviewed-by: Kevin Wolf
Reviewed-by: Jeff Cody
Message-id: 1477584421-1399-4-git-send-email-js...@redhat.com
Signed-off-by: Jeff Cody
---
block/backup.c| 3 ++-
block/mirror.c| 21 ++---
block/replication.c | 14 +++---
blockdev.c| 11 +++
include/block/block_int.h | 9 +++--
qemu-img.c| 5 +++--
6 files changed, 36 insertions(+), 27 deletions(-)
diff --git a/block/backup.c b/block/backup.c
index 3877d93..2a369e6 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -543,6 +543,7 @@ void backup_start(const char *job_id, BlockDriverState *bs,
bool compress,
BlockdevOnError on_source_error,
BlockdevOnError on_target_error,
+ int creation_flags,
BlockCompletionFunc *cb, void *opaque,
BlockJobTxn *txn, Error **errp)
{
@@ -612,7 +613,7 @@ void backup_start(const char *job_id, BlockDriverState *bs,
}
job = block_job_create(job_id, &backup_job_driver, bs, speed,
- BLOCK_JOB_DEFAULT, cb, opaque, errp);
+ creation_flags, cb, opaque, errp);
if (!job) {
goto error;
}
diff --git a/block/mirror.c b/block/mirror.c
index e9fba9b..fa41849 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -937,9 +937,9 @@ static const BlockJobDriver commit_active_job_driver = {
};
static void mirror_start_job(const char *job_id, BlockDriverState *bs,
- BlockDriverState *target, const char *replaces,
- int64_t speed, uint32_t granularity,
- int64_t buf_size,
+ int creation_flags, BlockDriverState *target,
+ const char *replaces, int64_t speed,
+ uint32_t granularity, int64_t buf_size,
BlockMirrorBackingMode backing_mode,
BlockdevOnError on_source_error,
BlockdevOnError on_target_error,
@@ -967,8 +967,8 @@ static void mirror_start_job(const char *job_id,
BlockDriverState *bs,
buf_size = DEFAULT_MIRROR_BUF_SIZE;
}
-s = block_job_create(job_id, driver, bs, speed,
- BLOCK_JOB_DEFAULT, cb, opaque, errp);
+s = block_job_create(job_id, driver, bs, speed, creation_flags,
+ cb, opaque, errp);
if (!s) {
return;
}
@@ -1031,17 +1031,16 @@ void mirror_start(const char *job_id, BlockDriverState
*bs,
}
is_none_mode = mode == MIRROR_SYNC_MODE_NONE;
base = mode == MIRROR_SYNC_MODE_TOP ? backing_bs(bs) : NULL;
-mirror_start_job(job_id, bs, target, replaces,
+mirror_start_job(job_id, bs, BLOCK_JOB_DEFAULT, target, replaces,
speed, granularity, buf_size, backing_mode,
on_source_error, on_target_error, unmap, cb, opaque, errp,
&mirror_job_driver, is_none_mode, base, false);
}
void commit_active_start(const char *job_id, BlockDriverState *bs,
- BlockDriverState *base, int64_t speed,
- BlockdevOnError on_error,
- BlockCompletionFunc *cb,
- void *opaque, Error **errp,
+ BlockDriverState *base, int creation_flags,
+ int64_t speed, BlockdevOnError on_error,
+ BlockCompletionFunc *cb, void *opaque, Error **errp,
bool auto_complete)
{
int64_t length, base_length;
@@ -1080,7 +1079,7 @@ void commit_active_start(const char *job_id,
BlockDriverState *bs,
}
}
-mirror_start_job(job_id, bs, base, NULL, speed, 0, 0,
+mirror_start_job(job_id, bs, creation_flags, base, NULL, speed, 0, 0,
MIRROR_LEAVE_BACKING_CHAIN,
on_error, on_error, true, cb, opaque, &local_err,
&commit_active_job_driver, false, base, auto_complete);
diff --git a/block/replication.c b/block/replication.c
index 02aeaaf..d5e2b0f 100644
--- a/block/replication.c
+++ b/block/replication.c
@@ -508,10 +508,11 @@ static void replication_start(ReplicationState *rs,
ReplicationMode mode,
bdrv_op_block_all(top_bs, s->blocker);
bdrv_op_unblock(top_bs, BLOCK_OP_TYPE_DATAPLANE, s->blocker);
-backup_start("replication-backup", s->secondary_disk->bs,
- s->hidden_disk->bs, 0, MIRROR_SYNC_MODE_NONE, NULL, false,
+backup_start(NULL, s->secondary_disk->bs, s->hidden_disk->bs, 0,
+ MIRROR_SYNC_MODE_NONE, NULL, false,
BLOCKDEV_ON_ERROR_REPORT, BLOC
[Qemu-devel] [PULL v2 11/14] blockjob: centralize QMP event emissions
From: John Snow
There's no reason to leave this to blockdev; we can do it in blockjobs
directly and get rid of an extra callback for most users.
All non-internal events, even those created outside of QMP, will
consistently emit events.
Signed-off-by: John Snow
Reviewed-by: Kevin Wolf
Reviewed-by: Jeff Cody
Message-id: 1477584421-1399-5-git-send-email-js...@redhat.com
Signed-off-by: Jeff Cody
---
block/commit.c| 8
block/mirror.c| 6 ++
block/stream.c| 7 +++
block/trace-events| 5 ++---
blockdev.c| 42 --
blockjob.c| 23 +++
include/block/block_int.h | 17 -
include/block/blockjob.h | 17 -
8 files changed, 42 insertions(+), 83 deletions(-)
diff --git a/block/commit.c b/block/commit.c
index 0740a41..18ec578 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -209,8 +209,8 @@ static const BlockJobDriver commit_job_driver = {
void commit_start(const char *job_id, BlockDriverState *bs,
BlockDriverState *base, BlockDriverState *top, int64_t speed,
- BlockdevOnError on_error, BlockCompletionFunc *cb,
- void *opaque, const char *backing_file_str, Error **errp)
+ BlockdevOnError on_error, const char *backing_file_str,
+ Error **errp)
{
CommitBlockJob *s;
BlockReopenQueue *reopen_queue = NULL;
@@ -234,7 +234,7 @@ void commit_start(const char *job_id, BlockDriverState *bs,
}
s = block_job_create(job_id, &commit_job_driver, bs, speed,
- BLOCK_JOB_DEFAULT, cb, opaque, errp);
+ BLOCK_JOB_DEFAULT, NULL, NULL, errp);
if (!s) {
return;
}
@@ -290,7 +290,7 @@ void commit_start(const char *job_id, BlockDriverState *bs,
s->on_error = on_error;
s->common.co = qemu_coroutine_create(commit_run, s);
-trace_commit_start(bs, base, top, s, s->common.co, opaque);
+trace_commit_start(bs, base, top, s, s->common.co);
qemu_coroutine_enter(s->common.co);
}
diff --git a/block/mirror.c b/block/mirror.c
index fa41849..aa60bcc 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -1018,9 +1018,7 @@ void mirror_start(const char *job_id, BlockDriverState
*bs,
MirrorSyncMode mode, BlockMirrorBackingMode backing_mode,
BlockdevOnError on_source_error,
BlockdevOnError on_target_error,
- bool unmap,
- BlockCompletionFunc *cb,
- void *opaque, Error **errp)
+ bool unmap, Error **errp)
{
bool is_none_mode;
BlockDriverState *base;
@@ -1033,7 +1031,7 @@ void mirror_start(const char *job_id, BlockDriverState
*bs,
base = mode == MIRROR_SYNC_MODE_TOP ? backing_bs(bs) : NULL;
mirror_start_job(job_id, bs, BLOCK_JOB_DEFAULT, target, replaces,
speed, granularity, buf_size, backing_mode,
- on_source_error, on_target_error, unmap, cb, opaque, errp,
+ on_source_error, on_target_error, unmap, NULL, NULL, errp,
&mirror_job_driver, is_none_mode, base, false);
}
diff --git a/block/stream.c b/block/stream.c
index 09ce9ef..152c1be 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -222,15 +222,14 @@ static const BlockJobDriver stream_job_driver = {
void stream_start(const char *job_id, BlockDriverState *bs,
BlockDriverState *base, const char *backing_file_str,
- int64_t speed, BlockdevOnError on_error,
- BlockCompletionFunc *cb, void *opaque, Error **errp)
+ int64_t speed, BlockdevOnError on_error, Error **errp)
{
StreamBlockJob *s;
BlockDriverState *iter;
int orig_bs_flags;
s = block_job_create(job_id, &stream_job_driver, bs, speed,
- BLOCK_JOB_DEFAULT, cb, opaque, errp);
+ BLOCK_JOB_DEFAULT, NULL, NULL, errp);
if (!s) {
return;
}
@@ -256,6 +255,6 @@ void stream_start(const char *job_id, BlockDriverState *bs,
s->on_error = on_error;
s->common.co = qemu_coroutine_create(stream_run, s);
-trace_stream_start(bs, base, s, s->common.co, opaque);
+trace_stream_start(bs, base, s, s->common.co);
qemu_coroutine_enter(s->common.co);
}
diff --git a/block/trace-events b/block/trace-events
index aff8a96..882c903 100644
--- a/block/trace-events
+++ b/block/trace-events
@@ -19,11 +19,11 @@ bdrv_co_do_copy_on_readv(void *bs, int64_t offset, unsigned
int bytes, int64_t c
# block/stream.c
stream_one_iteration(void *s, int64_t sector_num, int nb_sectors, int
is_allocated) "s %p sector_num %"PRId64" nb_sectors %d is_allocated %d"
-stream_start(void *bs, void *base, void *s, void *co, void *opaque) "bs %p
base %p s %p co %p opaque %p"
+stream_start(void *bs, void *ba
Re: [Qemu-devel] [PULL v2 00/14] Block patches for 2.8
Hi,
Your series seems to have some coding style problems. See output below for
more information:
Type: series
Subject: [Qemu-devel] [PULL v2 00/14] Block patches for 2.8
Message-id: 1478004671-19154-1-git-send-email-jc...@redhat.com
=== TEST SCRIPT BEGIN ===
#!/bin/bash
BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0
# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True
commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git show --no-patch --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done
exit $failed
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
* [new tag] patchew/1478004671-19154-1-git-send-email-jc...@redhat.com
-> patchew/1478004671-19154-1-git-send-email-jc...@redhat.com
- [tag update] patchew/58188cae.4a6ec20a.3d2d1.a...@mx.google.com ->
patchew/58188cae.4a6ec20a.3d2d1.a...@mx.google.com
Switched to a new branch 'test'
8e6d375 blockjobs: fix documentation
7142def blockjobs: split interface into public/private, Part 1
f8a014f Blockjobs: Internalize user_pause logic
3d3f46f blockjob: centralize QMP event emissions
ae0da88 Replication/Blockjobs: Create replication jobs as internal
be4560c blockjobs: Allow creating internal jobs
1da36d3 blockjobs: hide internal jobs from management API
1a65ed4 block/gluster: fix port type in the QAPI options list
718812b block/gluster: improve defense over string to int conversion
11dc993 block: Turn on "unmap" in active commit
5baafb5 block/gluster: memory usage: use one glfs instance per volume
63ea898 block: add gluster ifdef guard checks for SEEK_DATA/SEEK_HOLE support
9c11fb6 rbd: make the code more readable
8af9516 qapi: add release designator to gluster logfile option
=== OUTPUT BEGIN ===
fatal: unrecognized argument: --no-patch
Checking PATCH 1/14: ...
fatal: unrecognized argument: --no-patch
Checking PATCH 2/14: ...
fatal: unrecognized argument: --no-patch
Checking PATCH 3/14: ...
fatal: unrecognized argument: --no-patch
Checking PATCH 4/14: ...
fatal: unrecognized argument: --no-patch
Checking PATCH 5/14: ...
fatal: unrecognized argument: --no-patch
Checking PATCH 6/14: ...
fatal: unrecognized argument: --no-patch
Checking PATCH 7/14: ...
fatal: unrecognized argument: --no-patch
Checking PATCH 8/14: ...
fatal: unrecognized argument: --no-patch
Checking PATCH 9/14: ...
fatal: unrecognized argument: --no-patch
Checking PATCH 10/14: ...
fatal: unrecognized argument: --no-patch
Checking PATCH 11/14: ...
fatal: unrecognized argument: --no-patch
Checking PATCH 12/14: ...
fatal: unrecognized argument: --no-patch
Checking PATCH 13/14: ...
ERROR: struct BlockJobDriver should normally be const
#193: FILE: include/block/blockjob.h:31:
+typedef struct BlockJobDriver BlockJobDriver;
ERROR: struct BlockJobDriver should normally be const
#433: FILE: include/block/blockjob_int.h:37:
+struct BlockJobDriver {
ERROR: space prohibited between function name and open parenthesis '('
#477: FILE: include/block/blockjob_int.h:81:
+void coroutine_fn (*pause)(BlockJob *job);
ERROR: space prohibited between function name and open parenthesis '('
#484: FILE: include/block/blockjob_int.h:88:
+void coroutine_fn (*resume)(BlockJob *job);
total: 4 errors, 0 warnings, 578 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
fatal: unrecognized argument: --no-patch
Checking PATCH 14/14: ...
=== OUTPUT END ===
Test command exited with code: 1
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-de...@freelists.org
Re: [Qemu-devel] [PULL 00/47] virtio, pc: fixes and features
On Tue, 1 Nov 2016 00:48:11 +0200 "Michael S. Tsirkin" wrote: > On Mon, Oct 31, 2016 at 10:50:31AM +0100, Igor Mammedov wrote: > > On Sun, 30 Oct 2016 23:23:18 +0200 > > "Michael S. Tsirkin" wrote: > > > > > The following changes since commit > > > 5b2ecabaeabc17f032197246c4846b9ba95ba8a6: > > > > > > Merge remote-tracking branch 'remotes/kraxel/tags/pull-ui-20161028-1' > > > into staging (2016-10-28 17:59:04 +0100) > > > > > > are available in the git repository at: > > > > > > git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream > > > > > > for you to fetch changes up to f082ec0225bd15c71e0b4697d2df3af7bad65d7f: > > > > > > acpi: fix assert failure caused by commit 35c5a52d (2016-10-30 20:06:25 > > > +0200) > > > > > > > > > virtio, pc: fixes and features > > > > > > nvdimm hotplug support > > Michael, > > > > Could you drop nvdimm hotplug from pull request (I should review at least > > once as it touches not only NVDIMMs but a generic hotplug infrastructure) > > > > and keep only nvdimm fixes/cleanups for now? > > If I drop it now it won't be in the next QEMU and it seems like > a valuable feature. The comments so far are about minor style > improvements that IMO can be addressed by patches on top. wrt nvdimm hotplug support it's not about style issues but rather design issues: for example: - it extends general hotplug framework unnecessarily instead of figuring out how it works. - adds not needed locks maybe there is more and all of that was posted just a day before this pull request so I haven't even had a chance to review it properly. > We can always revert if you see bigger issues, but let's enable the > testing of this feature. if it didn't mess with general infrastructure, I wouldn't care much. But it does so I'd rather avoid merging not yet ready series just for the sake of getting it in. I haven't reviewed 28-35 patches either but they are all cleanups/ fixes of current nvdimm code and local to it so don't mind them getting merged. However I suggest dropping 36-39 patches from this pull request as not yet ready for merging. > > > > > virtio migration and ioeventfd rework > > > virtio crypto device > > > ipmi fixes > > > > > > Signed-off-by: Michael S. Tsirkin > > > > > > > > > Corey Minyard (5): > > > ipmi: Remove hotplug from IPMI BMCs > > > ipmi_bmc_sim: Remove an unnecessary mutex > > > ipmi: Implement shutdown via ACPI overtemp > > > ipmi: Add graceful shutdown handling to the external BMC > > > acpi/ipmi: Initialize the fwinfo before fetching it > > > > > > Cédric Le Goater (1): > > > ipmi: chassis poweroff should use qemu_system_shutdown_request() > > > > > > Daniel P. Berrange (1): > > > ipmi: fix build config variable name for ipmi_bmc_extern.o > > > > > > Dr. David Alan Gilbert (2): > > > virtio/migration: Add VMStateDescription to VirtioDeviceClass > > > virtio/migration: Migrate balloon to VMState > > > > > > Gonglei (12): > > > cryptodev: introduce cryptodev backend interface > > > cryptodev: add symmetric algorithm operation stuff > > > virtio-crypto: introduce virtio_crypto.h > > > cryptodev: introduce a new cryptodev backend > > > virtio-crypto: add virtio crypto device emulation > > > virtio-crypto-pci: add virtio crypto pci support > > > virtio-crypto: set capacity of algorithms supported > > > virtio-crypto: add control queue handler > > > virtio-crypto: add data queue processing handler > > > cryptodev: introduce an unified wrapper for crypto operation > > > virtio-crypto: using bh to handle dataq's requests > > > virtio-crypto: add myself as virtio-crypto and cryptodev backends > > > maintainer > > > > > > Haozhong Zhang (1): > > > acpi: fix assert failure caused by commit 35c5a52d > > > > > > Paolo Bonzini (13): > > > virtio: disable ioeventfd as early as possible > > > virtio: move ioeventfd_disabled flag to VirtioBusState > > > virtio: move ioeventfd_started flag to VirtioBusState > > > virtio: add start_ioeventfd and stop_ioeventfd to VirtioDeviceClass > > > virtio: introduce virtio_device_ioeventfd_enabled > > > virtio-blk: always use dataplane path if ioeventfd is active > > > virtio-scsi: always use dataplane path if ioeventfd is active > > > Revert "virtio: Introduce virtio_add_queue_aio" > > > virtio: remove set_handler argument from set_host_notifier_internal > > > virtio: remove ioeventfd_disabled altogether > > > virtio: use virtio_bus_set_host_notifier to start/stop ioeventfd > > > virtio: inline virtio_queue_set_host_notifier_fd_handler > > > virtio: inline set_host_notifier_internal > > > > > > Xiao Guangrong (12): > > > acpi nvdimm: fix wrong buffer
Re: [Qemu-devel] [PATCH V4] docs: add PCIe devices placement guidelines
On 10/31/2016 07:44 PM, Laine Stump wrote: On 10/31/2016 12:18 PM, Marcel Apfelbaum wrote: Hi Laine, Thank you for having a look at the document. + +2.2.1 Plugging a PCI Express device into a PCI Express Root Port: + -device ioh3420,id=root_port1,slot=x[,chassis=y][,bus=pcie.0][,addr=z] \ + -device ,bus=root_port1 + Note that (slot, chassis) pair is mandatory and must be + unique for each PCI Express Root Port. I keep meaning to ask about this and forgetting - just what is "slot" used for? In the past we were told that chassis and *port* were mandatory and must be unique, but hadn't before been told anything (that I can remember) about "slot": I am glad you asked, here is the PCIe Spec: 6.7.1.7. Slot Numbering A Physical Slot Identifier (as defined in PCI Hot-Plug Specification, Revision 1.1, Section 1.5) consists of an optional chassis number and the physical slot number of the slot. The physical slot number is a chassis unique identifier for a slot. System software determines the physical slot number from registers in the Port. Chassis number 0 is reserved for the main chassis. The chassis number for other chassis must be a non-zero value obtained from a PCI-to-PCI Bridge’s Chassis Number register (see the PCI-to-PCI Bridge Architecture Specification, Revision 1.2, Section 13.4). Regardless of the form factor associated with each slot, each physical slot number must be unique within a chassis. So we form the "Physical Slot Identifier" from both command line parameters "chassis" and "slot". The "Physical Slot Identifier" will by used as the method of identification of the PCI Express Root Port. Regarding the "port" command line parameter: 7.8.6. Link Capabilities Register (Offset 0Ch) 31:24 Port Number – This field indicates the PCI Express Port number for the given PCI Express Link. Multi-Function devices associated with an Upstream Port must report the same value in this field for all Functions. While it is set by QEMU, I don't think is has any significance for the emulated hardware since, as far as I understand it, the 'port' number is used for implementation of advanced hardware features as "Virtual Channels. So we can get away with leaving it 0... * If chassis isn't specified by the user, we will set it to the index of the controller (libvirt internally sets an index for each PCI controller, with the root bus being index 0, and each subsequent controller being the next higher number; this also is the number specified as "bus" in the libvirt config for the devices that connect to that controller (which is then used to determine the "id" of the controller used in the commandline argument for the device). As you can see, since we don't support more than 256 PCI Express Ports, we can get away with leaving the chassis 0 and increment the slot for each port. We don't support more ports because we support maximum 256 buses. Once we will have multiple PCI domains we'll need a different chassis per domain, but until then we are covered. * If port isn't specified by the user, then we set it according to where the port is attached in the root complex: port = slot << 3 + function I don't think is necessary. But what is slot? I hope the above explanation helps. +2.2.2 Using multi-function PCI Express Root Ports: + -device ioh3420,id=root_port1,multifunction=on,chassis=x[,bus=pcie.0][,slot=y][,addr=z.0] \ Similar to what Laszlo reported about v3 - these examples show slot as optional, where the first example shows it as mandatory. Also, none of the examples show "port" (which we were previously told was mandatory (or at least told that it needed to be unique) at all. Does the note from 2.2.1 specifying the pair should be unique help? Thinking about it more, maybe we should leave chassis optional because we don't support multiple PCI domains. I'll update the doc. + -device ioh3420,id=root_port2,chassis=x1[,bus=pcie.0][,slot=y1][,addr=z.1] \ + -device ioh3420,id=root_port3,chassis=x2[,bus=pcie.0][,slot=y2][,addr=z.2] \ +2.2.2 Plugging a PCI Express device into a Switch: + -device ioh3420,id=root_port1,chassis=x[,bus=pcie.0][,slot=y][,addr=z] \ + -device x3130-upstream,id=upstream_port1,bus=root_port1[,addr=x] \ + -device xio3130-downstream,id=downstream_port1,bus=upstream_port1,chassis=x1[,slot=y1][,addr=z1]] \ ..and there it is again, in downstream ports. + -device ,bus=downstream_port1 +Note that 'addr' parameter can be 0 for all the examples above. +Prefer flat hierarchies. For most scenarios a single DMI-PCI Bridge +(having 32 slots) and several PCI-PCI Bridges attached to it +(each supporting also 32 slots) will support hundreds of legacy devices. +The recommendation is to populate one PCI-
Re: [Qemu-devel] [PATCH v5 28/33] cputlb: make tlb_flush_by_mmuidx safe for MTTCG
On Tue, Nov 1, 2016 at 3:45 AM, Alex Bennée wrote: > > Odd. I'll look into it. What was you configure string and host architecture? > It's a plain configure string, nothing special: $ ../configure --target-list=aarch64-softmmu But I did rebase your patches on master, May be something new in the tree tripped this? -- Pranith
Re: [Qemu-devel] [PULL v2 for-2.8 0/7] 9p patches for 2.8 20161030
On 1 November 2016 at 12:00, Greg Kurz wrote: > The following changes since commit 02ba9265e8d65f24d0cdca158d96e0b0451f6b71: > > migration: fix compiler warning on uninitialized variable (2016-11-01 > 09:31:53 +) > > are available in the git repository at: > > https://github.com/gkurz/qemu.git tags/for-upstream > > for you to fetch changes up to 79decce35b4d769fa878b048ab1a7b3e9045c9c6: > > 9pfs: drop excessive error message from virtfs_reset() (2016-11-01 12:03:03 > +0100) > > > This pull request mostly contains some more fixes to prevent buggy guests from > breaking QEMU. > > v2: - added missing SoB tags > - rebased > > Greg Kurz (4): > 9pfs: limit xattr size in xattrcreate > 9pfs: xattrcreate requires non-opened fids > 9pfs: don't BUG_ON() if fid is already opened > 9pfs: drop excessive error message from virtfs_reset() > > Li Qiang (3): > 9pfs: add xattrwalk_fid field in V9fsXattr struct > 9pfs: convert 'len/copied_len' field in V9fsXattr to the type of > uint64_t > 9pfs: fix integer overflow issue in xattr read/write > > hw/9pfs/9p.c | 80 > +--- > hw/9pfs/9p.h | 5 ++-- > hw/9pfs/trace-events | 2 +- > 3 files changed, 49 insertions(+), 38 deletions(-) > -- Applied, thanks. -- PMM
[Qemu-devel] [PATCH V5] docs: add PCIe devices placement guidelines
Proposes best practices on how to use PCI Express/PCI device in PCI Express based machines and explain the reasoning behind them. Reviewed-by: Laszlo Ersek Signed-off-by: Marcel Apfelbaum --- Hi, v4->v5: - Addressed Laine's comments: - Advertize (slot,chassis) parameters as mandatory - Stated the Downstream ports are not hot-pluggable - Other minor typos v3->v4: - Addressed minor typos spotted by Laszlo, thanks! v2->v3: - Addressed the comments from Andrea Bolognani and Laszlo Ersek, which are much appreciated! - Added links to presentations that may help the understanding of the document. RFC->v2: - Addressed a lot of comments from the reviewers (many thanks to all, especially to Laszlo) Thanks, Marcel docs/pcie.txt | 311 ++ 1 file changed, 311 insertions(+) create mode 100644 docs/pcie.txt diff --git a/docs/pcie.txt b/docs/pcie.txt new file mode 100644 index 000..1d9bd18 --- /dev/null +++ b/docs/pcie.txt @@ -0,0 +1,311 @@ +PCI EXPRESS GUIDELINES +== + +1. Introduction + +The doc proposes best practices on how to use PCI Express/PCI device +in PCI Express based machines and explains the reasoning behind them. + +The following presentations accompany this document: + (1) Q35 overview. + http://wiki.qemu.org/images/4/4e/Q35.pdf + (2) A comparison between PCI and PCI Express technologies. + http://wiki.qemu.org/images/f/f6/PCIvsPCIe.pdf + +Note: The usage examples are not intended to replace the full +documentation, please use QEMU help to retrieve all options. + +2. Device placement strategy + +QEMU does not have a clear socket-device matching mechanism +and allows any PCI/PCI Express device to be plugged into any +PCI/PCI Express slot. +Plugging a PCI device into a PCI Express slot might not always work and +is weird anyway since it cannot be done for "bare metal". +Plugging a PCI Express device into a PCI slot will hide the Extended +Configuration Space thus is also not recommended. + +The recommendation is to separate the PCI Express and PCI hierarchies. +PCI Express devices should be plugged only into PCI Express Root Ports and +PCI Express Downstream ports. + +2.1 Root Bus (pcie.0) += +Place only the following kinds of devices directly on the Root Complex: +(1) PCI Devices (e.g. network card, graphics card, IDE controller), +not controllers. Place only legacy PCI devices on +the Root Complex. These will be considered Integrated Endpoints. +Note: Integrated Endpoints are not hot-pluggable. + +Although the PCI Express spec does not forbid PCI Express devices as +Integrated Endpoints, existing hardware mostly integrates legacy PCI +devices with the Root Complex. Guest OSes are suspected to behave +strangely when PCI Express devices are integrated +with the Root Complex. + +(2) PCI Express Root Ports (ioh3420), for starting exclusively PCI Express +hierarchies. + +(3) DMI-PCI Bridges (i82801b11-bridge), for starting legacy PCI +hierarchies. + +(4) Extra Root Complexes (pxb-pcie), if multiple PCI Express Root Buses +are needed. + + pcie.0 bus + +||| | + --- -- -- -- + | PCI Dev | | PCIe Root Port | | DMI-PCI Bridge | | pxb-pcie | + --- -- -- -- + +2.1.1 To plug a device into pcie.0 as a Root Complex Integrated Endpoint use: + -device [,bus=pcie.0] +2.1.2 To expose a new PCI Express Root Bus use: + -device pxb-pcie,id=pcie.1,bus_nr=x[,numa_node=y][,addr=z] + Only PCI Express Root Ports and DMI-PCI bridges can be connected + to the pcie.1 bus: + -device ioh3420,id=root_port1[,bus=pcie.1][,chassis=x][,slot=y][,addr=z] \ + -device i82801b11-bridge,id=dmi_pci_bridge1,bus=pcie.1 + + +2.2 PCI Express only hierarchy +== +Always use PCI Express Root Ports to start PCI Express hierarchies. + +A PCI Express Root bus supports up to 32 devices. Since each +PCI Express Root Port is a function and a multi-function +device may support up to 8 functions, the maximum possible +number of PCI Express Root Ports per PCI Express Root Bus is 256. + +Prefer grouping PCI Express Root Ports into multi-function devices +to keep a simple flat hierarchy that is enough for most scenarios. +Only use PCI Express Switches (x3130-upstream, xio3130-downstream) +if there is no more room for PCI Express Root Ports. +Please see section 4. for further justifications. + +Plug only PCI Express devices into PCI Express Ports. + + + pcie.0 bus +
Re: [Qemu-devel] [PULL 37/47] nvdimm acpi: introduce fit buffer
On 11/01/2016 06:35 PM, Igor Mammedov wrote:
On Tue, 1 Nov 2016 11:30:46 +0800
Xiao Guangrong wrote:
On 10/31/2016 07:09 PM, Igor Mammedov wrote:
On Mon, 31 Oct 2016 17:52:24 +0800
Xiao Guangrong wrote:
On 10/31/2016 05:45 PM, Igor Mammedov wrote:
On Sun, 30 Oct 2016 23:25:05 +0200
"Michael S. Tsirkin" wrote:
From: Xiao Guangrong
The buffer is used to save the FIT info for all the presented nvdimm
devices which is updated after the nvdimm device is plugged or
unplugged. In the later patch, it will be used to construct NVDIMM
ACPI _FIT method which reflects the presented nvdimm devices after
nvdimm hotplug
As FIT buffer can not completely mapped into guest address space,
OSPM will exit to QEMU multiple times, however, there is the race
condition - FIT may be changed during these multiple exits, so that
some rules are introduced:
1) the user should hold the @lock to access the buffer and
Could you explain why lock is needed?
Yes. These are two things need to be synced between QEMU and OSPM:
- fit buffer. QEMU products it and VM will consume it at the same time.
- dirty indicator. QEMU sets it and it will be cleared by OSPM, that means.
both sides will modify it.
I understand why 'dirty indicator' is necessary but
could you describe a concrete call flows in detail
that would cause concurrent access and need extra
lock protection.
Note that there is global lock (dubbed BQL) in QEMU,
which is taken every time guest accesses IO port or
QMP/monitor control event happens.
Ah. okay. If there is a lock to protect vm-exit handler and
monitor handler, i think it is okay to drop the lock.
2) mark @dirty whenever the buffer is updated.
@dirty is cleared for the first time OSPM gets fit buffer, if
dirty is detected in the later access, OSPM will restart the
access
As fit should be updated after nvdimm device is successfully realized
so that a new hotplug callback, post_hotplug, is introduced
Signed-off-by: Xiao Guangrong
Reviewed-by: Michael S. Tsirkin
Signed-off-by: Michael S. Tsirkin
---
include/hw/hotplug.h| 10 +
include/hw/mem/nvdimm.h | 26 +-
hw/acpi/nvdimm.c| 59 +++--
hw/core/hotplug.c | 11 +
hw/core/qdev.c | 20 +
hw/i386/acpi-build.c| 2 +-
hw/i386/pc.c| 19
7 files changed, 124 insertions(+), 23 deletions(-)
diff --git a/include/hw/hotplug.h b/include/hw/hotplug.h
index c0db869..10ca5b6 100644
--- a/include/hw/hotplug.h
+++ b/include/hw/hotplug.h
@@ -47,6 +47,7 @@ typedef void (*hotplug_fn)(HotplugHandler *plug_handler,
* @parent: Opaque parent interface.
* @pre_plug: pre plug callback called at start of device.realize(true)
* @plug: plug callback called at end of device.realize(true).
+ * @post_pug: post plug callback called after device is successfully plugged.
this doesn't seem to be necessary,
why hotplug_handler_plug() isn't sufficient?
At the point of hotplug_handler_plug(), the device is not realize (realized ==
0),
however, nvdimm_get_plugged_device_list() works on realized nvdimm devices.
I suggest instead of adding redundant hook, to reuse hotplug_handler_plug()
which is called after device::realize() successfully completed and amend
nvdimm_plugged_device_list() as follow:
diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index e486128..c6d8cbb 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -38,11 +38,7 @@ static int nvdimm_plugged_device_list(Object *obj, void
*opaque)
GSList **list = opaque;
if (object_dynamic_cast(obj, TYPE_NVDIMM)) {
-DeviceState *dev = DEVICE(obj);
-
-if (dev->realized) { /* only realized NVDIMMs matter */
-*list = g_slist_append(*list, DEVICE(obj));
-}
+*list = g_slist_append(*list, obj);
}
object_child_foreach(obj, nvdimm_plugged_device_list, opaque);
that should count not only already present nvdimms but also the one that's
being hotplugged.
It is not good as the device which failed to initialize
See device_set_realized()
[...]
if (dc->realize) {
dc->realize(dev, &local_err);
}
if (local_err != NULL) {
goto fail;
}
DEVICE_LISTENER_CALL(realize, Forward, dev);
if (hotplug_ctrl) {
hotplug_handler_plug(hotplug_ctrl, dev, &local_err);
}
i.e. control reaches to hotplug_handler_plug() only if
call dc->realize(dev, &local_err) is successful.
I mean, for example. if there are two devices, the first one is failed to be
realized, the second one is init-ed successfully, then can
nvdimm_plugged_device_list() get these two devices?
Based the code of pc_dimm_built_list(), i guess yes.
or is not being plugged
(consider two nvdimm devices directly appended in QEMU command line, when we
plug
the first one, both nvdimm devices will be counted in this list) is also
counted in
this list...
nope,
qd
Re: [Qemu-devel] [PULL 00/47] virtio, pc: fixes and features
On 11/01/2016 09:21 PM, Igor Mammedov wrote: On Tue, 1 Nov 2016 00:48:11 +0200 "Michael S. Tsirkin" wrote: On Mon, Oct 31, 2016 at 10:50:31AM +0100, Igor Mammedov wrote: On Sun, 30 Oct 2016 23:23:18 +0200 "Michael S. Tsirkin" wrote: The following changes since commit 5b2ecabaeabc17f032197246c4846b9ba95ba8a6: Merge remote-tracking branch 'remotes/kraxel/tags/pull-ui-20161028-1' into staging (2016-10-28 17:59:04 +0100) are available in the git repository at: git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream for you to fetch changes up to f082ec0225bd15c71e0b4697d2df3af7bad65d7f: acpi: fix assert failure caused by commit 35c5a52d (2016-10-30 20:06:25 +0200) virtio, pc: fixes and features nvdimm hotplug support Michael, Could you drop nvdimm hotplug from pull request (I should review at least once as it touches not only NVDIMMs but a generic hotplug infrastructure) and keep only nvdimm fixes/cleanups for now? If I drop it now it won't be in the next QEMU and it seems like a valuable feature. The comments so far are about minor style improvements that IMO can be addressed by patches on top. wrt nvdimm hotplug support it's not about style issues but rather design issues: for example: - it extends general hotplug framework unnecessarily instead of figuring out how it works. - adds not needed locks maybe there is more and all of that was posted just a day before this pull request so I haven't even had a chance to review it properly. We can always revert if you see bigger issues, but let's enable the testing of this feature. if it didn't mess with general infrastructure, I wouldn't care much. But it does so I'd rather avoid merging not yet ready series just for the sake of getting it in. I haven't reviewed 28-35 patches either but they are all cleanups/ fixes of current nvdimm code and local to it so don't mind them getting merged. However I suggest dropping 36-39 patches from this pull request as not yet ready for merging. Igor, Thank for your hard work to review this patchset. Only two patches related to general infrastructure that are: [PULL 37/47] nvdimm acpi: introduce fit buffer [PULL 39/47] pc: memhp: enable nvdimm device hotplug The issue you pointed out in [PULL 37/47] can be easily improved on the top of this patchset. Could you please look at the 39th, if there is no big issue, could it be merged first?
Re: [Qemu-devel] [PULL 00/47] virtio, pc: fixes and features
On Tue, Nov 01, 2016 at 02:21:07PM +0100, Igor Mammedov wrote: > On Tue, 1 Nov 2016 00:48:11 +0200 > "Michael S. Tsirkin" wrote: > > > On Mon, Oct 31, 2016 at 10:50:31AM +0100, Igor Mammedov wrote: > > > On Sun, 30 Oct 2016 23:23:18 +0200 > > > "Michael S. Tsirkin" wrote: > > > > > > > The following changes since commit > > > > 5b2ecabaeabc17f032197246c4846b9ba95ba8a6: > > > > > > > > Merge remote-tracking branch 'remotes/kraxel/tags/pull-ui-20161028-1' > > > > into staging (2016-10-28 17:59:04 +0100) > > > > > > > > are available in the git repository at: > > > > > > > > git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream > > > > > > > > for you to fetch changes up to f082ec0225bd15c71e0b4697d2df3af7bad65d7f: > > > > > > > > acpi: fix assert failure caused by commit 35c5a52d (2016-10-30 > > > > 20:06:25 +0200) > > > > > > > > > > > > virtio, pc: fixes and features > > > > > > > > nvdimm hotplug support > > > Michael, > > > > > > Could you drop nvdimm hotplug from pull request (I should review at least > > > once as it touches not only NVDIMMs but a generic hotplug infrastructure) > > > > > > and keep only nvdimm fixes/cleanups for now? > > > > If I drop it now it won't be in the next QEMU and it seems like > > a valuable feature. The comments so far are about minor style > > improvements that IMO can be addressed by patches on top. > wrt nvdimm hotplug support it's not about style issues but rather > design issues: for example: > - it extends general hotplug framework unnecessarily instead of >figuring out how it works. > - adds not needed locks > maybe there is more and all of that was posted just a day before > this pull request so I haven't even had a chance to review it properly. > > > > > We can always revert if you see bigger issues, but let's enable the > > testing of this feature. > if it didn't mess with general infrastructure, I wouldn't care much. > But it does so I'd rather avoid merging not yet ready series just for > the sake of getting it in. > > I haven't reviewed 28-35 patches either but they are all cleanups/ > fixes of current nvdimm code and local to it so don't mind them > getting merged. > > However I suggest dropping 36-39 patches from this pull request > as not yet ready for merging. So I think it's ok to keep them from now as that should help the feature converge faster, on the understanding the style issues are addressed quickly. Let's merge as is and I'll revert if this does not happen within two weeks. Ack? -- MST
Re: [Qemu-devel] [PULL 37/47] nvdimm acpi: introduce fit buffer
On Tue, 1 Nov 2016 21:40:46 +0800
Xiao Guangrong wrote:
> On 11/01/2016 06:35 PM, Igor Mammedov wrote:
> > On Tue, 1 Nov 2016 11:30:46 +0800
> > Xiao Guangrong wrote:
> >
> >> On 10/31/2016 07:09 PM, Igor Mammedov wrote:
> >>> On Mon, 31 Oct 2016 17:52:24 +0800
> >>> Xiao Guangrong wrote:
> >>>
> On 10/31/2016 05:45 PM, Igor Mammedov wrote:
> > On Sun, 30 Oct 2016 23:25:05 +0200
> > "Michael S. Tsirkin" wrote:
> >
> >> From: Xiao Guangrong
> >>
> >> The buffer is used to save the FIT info for all the presented nvdimm
> >> devices which is updated after the nvdimm device is plugged or
> >> unplugged. In the later patch, it will be used to construct NVDIMM
> >> ACPI _FIT method which reflects the presented nvdimm devices after
> >> nvdimm hotplug
> >>
> >> As FIT buffer can not completely mapped into guest address space,
> >> OSPM will exit to QEMU multiple times, however, there is the race
> >> condition - FIT may be changed during these multiple exits, so that
> >> some rules are introduced:
> >> 1) the user should hold the @lock to access the buffer and
> >>> Could you explain why lock is needed?
> >>
> >> Yes. These are two things need to be synced between QEMU and OSPM:
> >> - fit buffer. QEMU products it and VM will consume it at the same time.
> >> - dirty indicator. QEMU sets it and it will be cleared by OSPM, that means.
> >>both sides will modify it.
> >
> > I understand why 'dirty indicator' is necessary but
> > could you describe a concrete call flows in detail
> > that would cause concurrent access and need extra
> > lock protection.
> >
> > Note that there is global lock (dubbed BQL) in QEMU,
> > which is taken every time guest accesses IO port or
> > QMP/monitor control event happens.
>
> Ah. okay. If there is a lock to protect vm-exit handler and
> monitor handler, i think it is okay to drop the lock.
>
> >
> >> 2) mark @dirty whenever the buffer is updated.
> >>
> >> @dirty is cleared for the first time OSPM gets fit buffer, if
> >> dirty is detected in the later access, OSPM will restart the
> >> access
> >>
> >> As fit should be updated after nvdimm device is successfully realized
> >> so that a new hotplug callback, post_hotplug, is introduced
> >>
> >> Signed-off-by: Xiao Guangrong
> >> Reviewed-by: Michael S. Tsirkin
> >> Signed-off-by: Michael S. Tsirkin
> >> ---
> >> include/hw/hotplug.h| 10 +
> >> include/hw/mem/nvdimm.h | 26 +-
> >> hw/acpi/nvdimm.c| 59
> >> +++--
> >> hw/core/hotplug.c | 11 +
> >> hw/core/qdev.c | 20 +
> >> hw/i386/acpi-build.c| 2 +-
> >> hw/i386/pc.c| 19
> >> 7 files changed, 124 insertions(+), 23 deletions(-)
> >>
> >> diff --git a/include/hw/hotplug.h b/include/hw/hotplug.h
> >> index c0db869..10ca5b6 100644
> >> --- a/include/hw/hotplug.h
> >> +++ b/include/hw/hotplug.h
> >> @@ -47,6 +47,7 @@ typedef void (*hotplug_fn)(HotplugHandler
> >> *plug_handler,
> >> * @parent: Opaque parent interface.
> >> * @pre_plug: pre plug callback called at start of
> >> device.realize(true)
> >> * @plug: plug callback called at end of device.realize(true).
> >> + * @post_pug: post plug callback called after device is successfully
> >> plugged.
> > this doesn't seem to be necessary,
> > why hotplug_handler_plug() isn't sufficient?
>
> At the point of hotplug_handler_plug(), the device is not realize
> (realized == 0),
> however, nvdimm_get_plugged_device_list() works on realized nvdimm
> devices.
> >>>
> >>> I suggest instead of adding redundant hook, to reuse
> >>> hotplug_handler_plug()
> >>> which is called after device::realize() successfully completed and amend
> >>> nvdimm_plugged_device_list() as follow:
> >>>
> >>> diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
> >>> index e486128..c6d8cbb 100644
> >>> --- a/hw/acpi/nvdimm.c
> >>> +++ b/hw/acpi/nvdimm.c
> >>> @@ -38,11 +38,7 @@ static int nvdimm_plugged_device_list(Object *obj,
> >>> void *opaque)
> >>> GSList **list = opaque;
> >>>
> >>> if (object_dynamic_cast(obj, TYPE_NVDIMM)) {
> >>> -DeviceState *dev = DEVICE(obj);
> >>> -
> >>> -if (dev->realized) { /* only realized NVDIMMs matter */
> >>> -*list = g_slist_append(*list, DEVICE(obj));
> >>> -}
> >>> +*list = g_slist_append(*list, obj);
> >>> }
> >>>
> >>> object_child_foreach(obj, nvdimm_plugged_device_list, opaque);
> >>>
> >>> that should count not only already present nvdimms but also the one that's
> >>> being hotplugged.
> >>
> >> It is not good as the device which failed to initialize
> > See device_set_realized()
> > [...]
> >
Re: [Qemu-devel] [PATCH v9 04/12] vfio iommu: Add support for mediated devices
On 10/28/2016 7:48 AM, Alexey Kardashevskiy wrote: > On 27/10/16 23:31, Kirti Wankhede wrote: >> >> >> On 10/27/2016 12:50 PM, Alexey Kardashevskiy wrote: >>> On 18/10/16 08:22, Kirti Wankhede wrote: VFIO IOMMU drivers are designed for the devices which are IOMMU capable. Mediated device only uses IOMMU APIs, the underlying hardware can be managed by an IOMMU domain. Aim of this change is: - To use most of the code of TYPE1 IOMMU driver for mediated devices - To support direct assigned device and mediated device in single module Added two new callback functions to struct vfio_iommu_driver_ops. Backend IOMMU module that supports pining and unpinning pages for mdev devices should provide these functions. Added APIs for pining and unpining pages to VFIO module. These calls back into backend iommu module to actually pin and unpin pages. This change adds pin and unpin support for mediated device to TYPE1 IOMMU backend module. More details: - When iommu_group of mediated devices is attached, task structure is cached which is used later to pin pages and page accounting. >>> >>> >>> For SPAPR TCE IOMMU driver, I ended up caching mm_struct with >>> atomic_inc(&container->mm->mm_count) (patches are on the way) instead of >>> using @current or task as the process might be gone while VFIO container is >>> still alive and @mm might be needed to do proper cleanup; this might not be >>> an issue with this patchset now but still you seem to only use @mm from >>> task_struct. >>> >> >> Consider the example of QEMU process which creates VFIO container, QEMU >> in its teardown path would release the container. How could container be >> alive when process is gone? > > do_exit() in kernel/exit.c calls exit_mm() (which sets NULL to tsk->mm) > first, and then releases open files by calling exit_files(). So > container's release() does not have current->mm. > Incrementing usage count (get_task_struct()) while saving task structure and decementing it (put_task_struct()) from release() should work here. Updating the patch. Thanks, Kirti
Re: [Qemu-devel] [PULL 00/47] virtio, pc: fixes and features
On Tue, 1 Nov 2016 15:55:36 +0200 "Michael S. Tsirkin" wrote: > On Tue, Nov 01, 2016 at 02:21:07PM +0100, Igor Mammedov wrote: > > On Tue, 1 Nov 2016 00:48:11 +0200 > > "Michael S. Tsirkin" wrote: > > > > > On Mon, Oct 31, 2016 at 10:50:31AM +0100, Igor Mammedov wrote: > > > > On Sun, 30 Oct 2016 23:23:18 +0200 > > > > "Michael S. Tsirkin" wrote: > > > > > > > > > The following changes since commit > > > > > 5b2ecabaeabc17f032197246c4846b9ba95ba8a6: > > > > > > > > > > Merge remote-tracking branch > > > > > 'remotes/kraxel/tags/pull-ui-20161028-1' into staging (2016-10-28 > > > > > 17:59:04 +0100) > > > > > > > > > > are available in the git repository at: > > > > > > > > > > git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream > > > > > > > > > > for you to fetch changes up to > > > > > f082ec0225bd15c71e0b4697d2df3af7bad65d7f: > > > > > > > > > > acpi: fix assert failure caused by commit 35c5a52d (2016-10-30 > > > > > 20:06:25 +0200) > > > > > > > > > > > > > > > virtio, pc: fixes and features > > > > > > > > > > nvdimm hotplug support > > > > Michael, > > > > > > > > Could you drop nvdimm hotplug from pull request (I should review at > > > > least once as it touches not only NVDIMMs but a generic hotplug > > > > infrastructure) > > > > > > > > and keep only nvdimm fixes/cleanups for now? > > > > > > If I drop it now it won't be in the next QEMU and it seems like > > > a valuable feature. The comments so far are about minor style > > > improvements that IMO can be addressed by patches on top. > > wrt nvdimm hotplug support it's not about style issues but rather > > design issues: for example: > > - it extends general hotplug framework unnecessarily instead of > >figuring out how it works. > > - adds not needed locks > > maybe there is more and all of that was posted just a day before > > this pull request so I haven't even had a chance to review it properly. > > > > > > > > > We can always revert if you see bigger issues, but let's enable the > > > testing of this feature. > > if it didn't mess with general infrastructure, I wouldn't care much. > > But it does so I'd rather avoid merging not yet ready series just for > > the sake of getting it in. > > > > I haven't reviewed 28-35 patches either but they are all cleanups/ > > fixes of current nvdimm code and local to it so don't mind them > > getting merged. > > > > However I suggest dropping 36-39 patches from this pull request > > as not yet ready for merging. > > So I think it's ok to keep them from now as that should help > the feature converge faster, on the understanding the > style issues are addressed quickly. > > Let's merge as is and I'll revert if this does not happen > within two weeks. Ack? Ack, let's merge and revert if author won't fix issues in 1-2 weeks. PS: Looks like with new soft-freeze rules we just have one big hard-freeze window and people trying to frantically squeeze in features last minute. I think previous soft-freeze rules worked better where maintainers were still allowed merge features at their discretion if series would converge in soft-freeze time frame and be important/low risk one. Like the case here.
Re: [Qemu-devel] [PULL 08/27] hostmem-file: make option 'size' optional
On Tue, Nov 01, 2016 at 05:32:20PM +0800, Haozhong Zhang wrote:
> On 10/31/16 20:22 -0200, Eduardo Habkost wrote:
> > On Mon, Oct 31, 2016 at 03:47:53PM -0400, Paolo Bonzini wrote:
> > >
> > >
> > > - Original Message -
> > > > From: "Eduardo Habkost"
> > > > To: "Paolo Bonzini"
> > > > Cc: qemu-devel@nongnu.org, "Haozhong Zhang"
> > > > Sent: Monday, October 31, 2016 7:20:10 PM
> > > > Subject: Re: [Qemu-devel] [PULL 08/27] hostmem-file: make option 'size'
> > > > optional
> > > >
> > > > On Mon, Oct 31, 2016 at 03:37:24PM +0100, Paolo Bonzini wrote:
> > > > [...]
> > > > > @@ -1309,21 +1317,27 @@ static void *file_ram_alloc(RAMBlock *block,
> > > > >
> > > > > file_size = get_file_size(fd);
> > > > >
> > > > > -if (memory < block->page_size) {
> > > > > +if (!mem_size && file_size > 0) {
> > > > > +mem_size = file_size;
> > > > > +memory_region_set_size(block->mr, mem_size);
> > > > > +}
> > > > > +
> > > > > +if (mem_size < block->page_size) {
> > > > > error_setg(errp, "memory size 0x" RAM_ADDR_FMT " must be
> > > > > equal to
> > > > > "
> > > > > "or larger than page size 0x%zx",
> > > > > - memory, block->page_size);
> > > > > + mem_size, block->page_size);
> > > > > goto error;
> > > > > }
> > > > >
> > > > > -if (file_size > 0 && file_size < memory) {
> > > > > +if (file_size > 0 && file_size < mem_size) {
> > > > > error_setg(errp, "backing store %s size %"PRId64
> > > > > " does not match 'size' option %"PRIu64,
> > > > > - path, file_size, memory);
> > > > > + path, file_size, mem_size);
> > > > > goto error;
> > > > > }
> > > > >
> > > > > -memory = ROUND_UP(memory, block->page_size);
> > > > > +mem_size = ROUND_UP(mem_size, block->page_size);
> > > > > +*memory = mem_size;
> > > >
> > > > I suggested not touching *memory unless it was zero, and setting
> > > > it to the memory region size, not the rounded-up size. Haozhong
> > > > said this was going to be changed.
> > > >
> > > > This will have the side-effect of setting block->used_length and
> > > > block->max_length to the rounded up size in
> > > > qemu_ram_alloc_from_file() (instead of the original memory region
> > > > size). I don't know what could be the consequences of that.
> > > >
> > > > This patch also skip HOST_PAGE_ALIGN-ing mem_size after getting
> > > > the file size, which would be different from the behavior when
> > > > size is specified explicitly. (And I also don't know the
> > > > consequences of that)
> > > >
> > > > Considering that this pull request failed to build, I suggest
> > > > waiting for a new version from Haozhong.
> > >
> > > Yes, I'll drop these three patches.
> >
> > I believe you can keep the other two (as long as the build error
> > is fixed). I was already going to include them in my pull
> > request, but removed them.
>
> I'm a little confused. Do I need to send a following patch to fix this
> build error? I was going to send a new version of the entire patch
> series.
I thought we could fix it while committing, to make sure it gets
in a pull request for soft freeze. But:
* Patch 1/3 ("do not truncate") is a bug fix for nvdimm, so
eligible post-soft-freeze.
* Patch 2/3 ("check file size") looks like a bug fix too.
* Patch 3/3 ("make size optional") is not a bug fix and is
riskier, so I believe it won't get into 2.8.
So sending a new series is probably simpler, as patch 1-2 can be
included after soft freeze.
--
Eduardo
Re: [Qemu-devel] [PULL v2 for-2.8 0/9] tcg queued patches
On 1 November 2016 at 12:07, Richard Henderson wrote: > V2, now with more windows workarounds. I'll note that I have not > been 100% successful in actually building a mingw64 image. But > at least the error Peter saw with v1 is fixed. > > I'll report on the other mingw64 failures under separate cover. > > > r~ > > > > The following changes since commit e80b4b8fb6babce7dcc91ea9ddeecbc351fd4646: > > Merge remote-tracking branch > 'remotes/awilliam/tags/vfio-updates-20161031.0' into staging (2016-10-31 > 18:19:06 +) > > are available in the git repository at: > > git://github.com/rth7680/qemu.git tags/pull-tcg-20161101 > > for you to fetch changes up to 13110e2d41d0b9d77edea5b23e1a808c86eb22cb: > > tcg: correct 32-bit tcg_gen_ld8s_i64 sign-extension (2016-10-31 21:36:29 > -0600) > > > tcg queued patches i686-w64-mingw32 builds fail to link :-( exec.o: In function `qemu_flockfile': /home/petmay01/linaro/qemu-for-merges/include/sysemu/os-win32.h:110: undefined reference to `_imp___lock_file' exec.o: In function `qemu_funlockfile': /home/petmay01/linaro/qemu-for-merges/include/sysemu/os-win32.h:115: undefined reference to `_imp___unlock_file' (repeated a lot of times) thanks -- PMM
Re: [Qemu-devel] [PATCH] migration: fix missing assignment for has_x_checkpoint_delay
On 11/01/2016 12:50 AM, zhanghailiang wrote: > We forgot to assign true to params->has_x_checkpoint_delay parameter > in qmp_query_migrate_parameters. > > Without this, qmp command 'query-migrate-parameters' doesn't show the > default value for x-checkpoint-delay option. > It doesn't influence output of hmp command 'info migrate_parameters'. Well, only because the current code doesn't forcefully assign missing optional parameters to any other value. But HMP was relying on unspecified behavior, that could have broken with any other qapi change. I might word the commit message: This also fixes the fact that HMP was relying on unspecified behavior by reading x_checkpoint_delay without checking has_x_checkpoint_delay. Up to the maintainer, though, since the patch itself is fine. > > Signed-off-by: zhanghailiang > --- > hmp.c | 1 + > migration/migration.c | 1 + > 2 files changed, 2 insertions(+) Reviewed-by: Eric Blake -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PULL 00/47] virtio, pc: fixes and features
On 1 November 2016 at 14:14, Igor Mammedov wrote: > PS: > Looks like with new soft-freeze rules we just have one big > hard-freeze window and people trying to frantically squeeze > in features last minute. > I think previous soft-freeze rules worked better where > maintainers were still allowed merge features at their > discretion if series would converge in soft-freeze time > frame and be important/low risk one. Like the case here. But you get an extra week before softfreeze deadline (ie softfreeze is 2 weeks rather than 3), so it evens out a bit. At a week into old-style softfreeze you as a submaintainer should probably have been pushing back on this as late for a new feature anyway. thanks -- PMM
[Qemu-devel] [PATCH] atomic.h: Use __atomic_load_n() for acquire
We can use __atomic_load_n() saving a store and load from the _val.
Signed-off-by: Pranith Kumar
---
include/qemu/atomic.h | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/include/qemu/atomic.h b/include/qemu/atomic.h
index 878fa07..6775603 100644
--- a/include/qemu/atomic.h
+++ b/include/qemu/atomic.h
@@ -144,9 +144,7 @@
#define atomic_load_acquire(ptr)\
({ \
QEMU_BUILD_BUG_ON(sizeof(*ptr) > sizeof(void *)); \
-typeof_strip_qual(*ptr) _val; \
-__atomic_load(ptr, &_val, __ATOMIC_ACQUIRE);\
-_val; \
+__atomic_load_n(ptr, __ATOMIC_ACQUIRE); \
})
#define atomic_store_release(ptr, i) do { \
--
2.10.2
[Qemu-devel] [PATCH 1/1] checkpatch: allow spaces before parenthesis for 'coroutine_fn'
Signed-off-by: Jeff Cody
---
scripts/checkpatch.pl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 3afa19a..9e64e61 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1754,7 +1754,7 @@ sub process {
# Ignore those directives where spaces _are_ permitted.
if ($name =~ /^(?:
if|for|while|switch|return|case|
- volatile|__volatile__|
+ volatile|__volatile__|coroutine_fn|
__attribute__|format|__extension__|
asm|__asm__)$/x)
{
--
2.7.4
Re: [Qemu-devel] [PATCH 1/1] checkpatch: allow spaces before parenthesis for 'coroutine_fn'
On 11/01/2016 09:38 AM, Jeff Cody wrote:
> Signed-off-by: Jeff Cody
> ---
> scripts/checkpatch.pl | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Eric Blake
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 3afa19a..9e64e61 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -1754,7 +1754,7 @@ sub process {
> # Ignore those directives where spaces _are_ permitted.
> if ($name =~ /^(?:
> if|for|while|switch|return|case|
> - volatile|__volatile__|
> + volatile|__volatile__|coroutine_fn|
> __attribute__|format|__extension__|
> asm|__asm__)$/x)
> {
>
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] atomic.h: Use __atomic_load_n() for acquire
On 01/11/2016 15:33, Pranith Kumar wrote:
> We can use __atomic_load_n() saving a store and load from the _val.
>
> Signed-off-by: Pranith Kumar
> ---
> include/qemu/atomic.h | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/include/qemu/atomic.h b/include/qemu/atomic.h
> index 878fa07..6775603 100644
> --- a/include/qemu/atomic.h
> +++ b/include/qemu/atomic.h
> @@ -144,9 +144,7 @@
> #define atomic_load_acquire(ptr)\
> ({ \
> QEMU_BUILD_BUG_ON(sizeof(*ptr) > sizeof(void *)); \
> -typeof_strip_qual(*ptr) _val; \
> -__atomic_load(ptr, &_val, __ATOMIC_ACQUIRE);\
> -_val; \
> +__atomic_load_n(ptr, __ATOMIC_ACQUIRE); \
> })
>
> #define atomic_store_release(ptr, i) do { \
>
Can you do the same for atomic_rcu_read__nocheck (both implementations)?
Thanks,
Paolo
Re: [Qemu-devel] [PATCH] atomic.h: Use __atomic_load_n() for acquire
On Tue, Nov 1, 2016 at 10:44 AM, Paolo Bonzini wrote: > > Can you do the same for atomic_rcu_read__nocheck (both implementations)? > Sure, will send an updated patch. -- Pranith
Re: [Qemu-devel] [PATCH 1/1] checkpatch: allow spaces before parenthesis for 'coroutine_fn'
On 01/11/2016 15:38, Jeff Cody wrote:
> Signed-off-by: Jeff Cody
> ---
> scripts/checkpatch.pl | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 3afa19a..9e64e61 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -1754,7 +1754,7 @@ sub process {
> # Ignore those directives where spaces _are_ permitted.
> if ($name =~ /^(?:
> if|for|while|switch|return|case|
> - volatile|__volatile__|
> + volatile|__volatile__|coroutine_fn|
> __attribute__|format|__extension__|
> asm|__asm__)$/x)
> {
>
Looks good, thanks.
Paolo
Re: [Qemu-devel] [PATCH 8/8] nvdimm acpi: use common macros instead of magic names
On Sat, Oct 29, 2016 at 12:11:56AM +0800, Xiao Guangrong wrote: > /* > * DSM notifier: > - * NTFI: write the address of DSM memory and notify QEMU to emulate > - * the access. > + * NVDIMM_DSM_NOTIFY: write the address of DSM memory and notify QEMU to > + *emulate the access. Did you mean to search-replace this instance of "NTFI"? > /* > * DSM input: > - * HDLE: store device's handle, it's zero if the _DSM call happens > - * on NVDIMM Root Device. > - * REVS: store the Arg1 of _DSM call. > - * FUNC: store the Arg2 of _DSM call. > - * FARG: store the Arg3 of _DSM call which is a Package containing > - * function-specific arguments. > + * NVDIMM_DSM_HANDLE: store device's handle, it's zero if the _DSM call > + *happens on NVDIMM Root Device. > + * NVDIMM_DSM_REVISION: store the Arg1 of _DSM call. > + * NVDIMM_DSM_FUNCTION: store the Arg2 of _DSM call. > + * NVDIMM_DSM_ARG3: store the Arg3 of _DSM call which is a Package > + * containing function-specific arguments. Did you mean to search-replace these names? I think it makes sense to use the literal ACPI names instead of the macro names in documentation. > /* > * DSM output: > - * RLEN: the size of the buffer filled by QEMU. > - * ODAT: the buffer QEMU uses to store the result. > + * NVDIMM_DSM_OUT_BUF_SIZE: the size of the buffer filled by QEMU. > + * NVDIMM_DSM_OUT_BUF: the buffer QEMU uses to store the result. Same here. signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH 0/8] nvdimm acpi: bug fix and cleanup
On Sat, Oct 29, 2016 at 12:11:48AM +0800, Xiao Guangrong wrote: > Thanks to Igor's suggestion, this patchset fixes some bugs in NVDIMM ACPI, > also it refines the nvdimm code slightly > > Xiao Guangrong (8): > acpi nvdimm: fix wrong buffer size returned by DSM method > acpi nvdimm: fix device physical address base > acpi nvdimm: fix OperationRegion definition > acpi nvdimm: fix ARG3 conflict > acpi nvdimm: fix Arg6 usage > nvdimm acpi: compile nvdimm acpi code arch-independently > acpi nvdimm: rename result_size to dsm_out_buf_siz > nvdimm acpi: use common macros instead of magic names > > hw/acpi/Makefile.objs | 2 +- > hw/acpi/nvdimm.c | 180 > -- > 2 files changed, 101 insertions(+), 81 deletions(-) Besides my comments on the last patch: Reviewed-by: Stefan Hajnoczi signature.asc Description: PGP signature
Re: [Qemu-devel] [QEMU PATCH v9 2/3] migration: migrate QTAILQ
On 31/10/2016 14:10, Halil Pasic wrote: > I think this got overly complicated. I agree. :) > Here is a little patch on > top of your stuff which gets rid of 15 lines and IMHO simplifies > things quite a bit. What do you think? > > It is based on/inspired by Dave's proposal with the dummy stuff. > > Did not address the typos though. I still prefer my field_at_offset proposal, but I'm obviously biased. Squashing your changes on top of 2/3 is fine too. But this change makes little sense: > */ > #define QTAILQ_RAW_NEXT(elm, entry) > \ > -(*field_at_offset(elm, entry + QTAILQ_NEXT_OFFSET, void **)) > +((field_at_offset(elm, entry, QTAILQRawEntry *)->tqe_next)) > #define QTAILQ_RAW_PREV(elm, entry) > \ > -(*field_at_offset(elm, entry + QTAILQ_PREV_OFFSET, void ***)) > +(field_at_offset(elm, entry, QTAILQRawEntry *)->tqe_prev) > + field_at_offset seems to be out of place. Paolo
Re: [Qemu-devel] [PATCH] cpus: make qemu_mutex_iothread_locked() understand co-routines
On 21/10/2016 13:54, Alex Bennée wrote:
> There is a slight wart when checking for the state of the BQL when using
> GThread base co-routines (which we keep for ThreadSanitizer runs). While
> the main-loop holds the BQL it is suspended until the co-routine
> completes however the co-routines run in a separate thread so checking
> the TLS variable could be wrong.
>
> We fix this by expanding the check to include qemu_in_coroutine() for
> GThread based builds. As it is not used for production builds I'm not
> overly worried about any performance impact which should be negligible
> anyway.
>
> Signed-off-by: Alex Bennée
> Cc: Stefan Hajnoczi
This is wrong unfortunately. It is possible to run coroutines outside
the BQL (e.g. with -device virtio-blk,iothread=foo).
Do you know exactly why TSAN has no love for coroutines?
Paolo
> ---
> configure | 3 +++
> cpus.c| 13 +
> 2 files changed, 16 insertions(+)
>
> diff --git a/configure b/configure
> index 91a14c1..97b89fb 100755
> --- a/configure
> +++ b/configure
> @@ -5461,6 +5461,9 @@ if test "$rbd" = "yes" ; then
> fi
>
> echo "CONFIG_COROUTINE_BACKEND=$coroutine" >> $config_host_mak
> +if test "$coroutine" = "gthread" ; then
> + echo "CONFIG_COROUTINE_GTHREAD=1" >> $config_host_mak
> +fi
> if test "$coroutine_pool" = "yes" ; then
>echo "CONFIG_COROUTINE_POOL=1" >> $config_host_mak
> else
> diff --git a/cpus.c b/cpus.c
> index 0c18a9f..a3e189a 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -49,6 +49,10 @@
> #include "hw/nmi.h"
> #include "sysemu/replay.h"
>
> +#ifdef CONFIG_COROUTINE_GTHREAD
> +#include "qemu/coroutine.h"
> +#endif
> +
> #ifndef _WIN32
> #include "qemu/compatfd.h"
> #endif
> @@ -1422,9 +1426,18 @@ bool qemu_in_vcpu_thread(void)
>
> static __thread bool iothread_locked = false;
>
> +/*
> + * There is a slight wart when using gthread based co-routines. Here
> + * the BQL is held by the main-thread which is suspended until the
> + * co-routines complete.
> + */
> bool qemu_mutex_iothread_locked(void)
> {
> +#ifdef CONFIG_COROUTINE_GTHREAD
> +return iothread_locked || qemu_in_coroutine();
> +#else
> return iothread_locked;
> +#endif
> }
>
> void qemu_mutex_lock_iothread(void)
>
Re: [Qemu-devel] [PATCH] docs/rcu.txt: Fix minor typo
On 18/10/2016 07:04, Pranith Kumar wrote: > s/presented/prevented/ > > Signed-off-by: Pranith Kumar > --- > docs/rcu.txt | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/docs/rcu.txt b/docs/rcu.txt > index a70b72c..c84e7f4 100644 > --- a/docs/rcu.txt > +++ b/docs/rcu.txt > @@ -145,7 +145,7 @@ The core RCU API is small: > and then read from there. > > RCU read-side critical sections must use atomic_rcu_read() to > -read data, unless concurrent writes are presented by another > +read data, unless concurrent writes are prevented by another > synchronization mechanism. > > Furthermore, RCU read-side critical sections should traverse the > Finally queued, thanks. Paolo
Re: [Qemu-devel] [PATCH] main-loop: Suppress I/O thread warning under qtest
On 17/10/2016 20:09, Max Reitz wrote:
> We do not want to display the "I/O thread spun" warning for test cases
> that run under qtest. The first attempt for this (commit
> 01c22f2cdd4fcf02276ea10f48253850a5fd7259) tested whether qtest_enabled()
> was true.
>
> Commit 21a24302e85024dd7b2a151158adbc1f5dc5c4dd correctly recognized
> that just testing qtest_enabled() is not sufficient since there are some
> tests that do not use the qtest accelerator but just the qtest character
> device, and thus replaced qtest_enabled() by qtest_driver().
>
> However, there are also some tests that only use the qtest accelerator
> and not the qtest chardev; perhaps most notably the bash iotests.
> Therefore, we have to check both qtest_enabled() and qtest_driver().
>
> Signed-off-by: Max Reitz
> ---
> main-loop.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/main-loop.c b/main-loop.c
> index 6a7f8d3..889b5bf 100644
> --- a/main-loop.c
> +++ b/main-loop.c
> @@ -232,7 +232,7 @@ static int os_host_main_loop_wait(int64_t timeout)
> if (!timeout && (spin_counter > MAX_MAIN_LOOP_SPIN)) {
> static bool notified;
>
> -if (!notified && !qtest_driver()) {
> +if (!notified && !qtest_enabled() && !qtest_driver()) {
> fprintf(stderr,
> "main-loop: WARNING: I/O thread spun for %d
> iterations\n",
> MAX_MAIN_LOOP_SPIN);
>
Finally queued, thanks.
Paolo
Re: [Qemu-devel] [PULL v2 00/14] Block patches for 2.8
On 1 November 2016 at 12:50, Jeff Cody wrote: > The following changes since commit bf99fd3983d7185178a0f65ce29bb94b1aecaed1: > > Merge remote-tracking branch 'remotes/rth/tags/pull-sparc-20161031-2' into > staging (2016-11-01 11:21:02 +) > > are available in the git repository at: > > g...@github.com:codyprime/qemu-kvm-jtc.git tags/block-pull-request > > for you to fetch changes up to d8996368106fbf133a6e52561a34f6d0f5080446: > > blockjobs: fix documentation (2016-11-01 08:04:56 -0400) > > > Block patches for 2.8 > > > Fam Zheng (1): > block: Turn on "unmap" in active commit > > Jeff Cody (2): > qapi: add release designator to gluster logfile option > block: add gluster ifdef guard checks for SEEK_DATA/SEEK_HOLE support > > John Snow (7): > blockjobs: hide internal jobs from management API > blockjobs: Allow creating internal jobs > Replication/Blockjobs: Create replication jobs as internal > blockjob: centralize QMP event emissions > Blockjobs: Internalize user_pause logic > blockjobs: split interface into public/private, Part 1 > blockjobs: fix documentation > > Prasanna Kumar Kalever (3): > block/gluster: memory usage: use one glfs instance per volume > block/gluster: improve defense over string to int conversion > block/gluster: fix port type in the QAPI options list > > Xiubo Li (1): > rbd: make the code more readable Applied, thanks. -- PMM
