On 5/31/22, 7:29 AM, "Michal Prívozník" <mprivozn@redhat.com> wrote:
> So, now that I've cleaned up the code I can start to test it, but
> unfortunately, I don't have good news. It's not working. I've put the
> following into my domain XML:
>
> <memory model='sgx-epc'>
> <target>
> <size unit='KiB'>16384</size>
> </target>
> </memory>
>
> and this is the generated cmd line:
>
> -machine
> pc-i440fx-6.2,usb=off,dump-guest-core=off,sgx-epc.0.memdev=memepc0 \
>
> -object
> '{"qom-type":"memory-backend-memfd","id":"memepc0","hugetlb":true,"hugetlbsize":2097152,"share":true,"prealloc":true,> "prealloc-threads":16,"size":16777216,"host-nodes":[0],"policy":"bind"}'
> \
>
> but if fails with:
>
> 2022-05-31T14:05:22.988793Z qemu-system-x86_64: Parameter
> 'sgx-epc.0.node' is missing
>
> Now, there are two problems here:
>
> 1) obviously, wrong backend was picked. But this is easy to solve - just
> move the if (mem->model == VIR_DOMAIN_MEMORY_MODEL_SGX_EPC) case in
> qemuBuildMemoryBackendProps() from the last patch before the memfd case.
>
Thank you so much for resolving this issue.
> 2) apparently, .node attribute is required? Now, it's true that
> initially my guest has 2 NUMA nodes defined, but even after I remove
> those I still see the error. I believe I've raised this issue in one of
> earlier reviews:
>
> https://listman.redhat.com/archives/libvir-list/2022-February/228835.html
>
> Please make sure that in v13 this is addressed (even at expense of not
> working with QEMU-6.2.0 and requiring newer QEMU, if that's needed).
> Wasting precious reviewer bandwidth does not help anybody.
The .node attribute is required for qemu 7.0.0, but qemu 6.2.0 doesn’t need
It. Missing .node will fail with qemu 7.0.0, even only define one NUMA node.
Giving it’s probably impossible to detect whether .node is needed or not, our
proposal here is this patch only works with qemu 6.2.0, then support qemu 7.0.0
in another patch. It might give user a chance to choose different libvirt version,
or pick up some commits to work with different qemu version.
If it’s unnecessary, we can definitely only support 7.0.0 as you suggested.
Thanks,
Lin.