We already calculated the guest area, which is what is subject
to minimum size requirements, a few lines earlier.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
---
 src/qemu/qemu_domain.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index f14a15d3b4..4d007bc4a0 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -5363,7 +5363,7 @@ qemuDomainNVDimmAlignSizePseries(virDomainMemoryDefPtr mem)
     /* Align down guest_area. 256MiB is the minimum size. Error
      * out if target_size is smaller than 256MiB + label_size,
      * since aligning it up will cause QEMU errors. */
-    if (mem->size < (ppc64AlignSize + mem->labelsize)) {
+    if (guestArea < ppc64AlignSize) {
         virReportError(VIR_ERR_XML_ERROR, "%s",
                        _("minimum target size for the NVDIMM "
                          "must be 256MB plus the label size"));
-- 
2.26.2

On 12/7/20 2:07 PM, Andrea Bolognani wrote:
> We already calculated the guest area, which is what is subject
> to minimum size requirements, a few lines earlier.
> 
> Signed-off-by: Andrea Bolognani <abologna@redhat.com>
> ---
>   src/qemu/qemu_domain.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index f14a15d3b4..4d007bc4a0 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -5363,7 +5363,7 @@ qemuDomainNVDimmAlignSizePseries(virDomainMemoryDefPtr mem)
>       /* Align down guest_area. 256MiB is the minimum size. Error
>        * out if target_size is smaller than 256MiB + label_size,
>        * since aligning it up will cause QEMU errors. */
> -    if (mem->size < (ppc64AlignSize + mem->labelsize)) {
> +    if (guestArea < ppc64AlignSize) {

Makes sense. I suggest to simplify the comment right above it as well:


        /* Align down guest_area. We can't align down if guest_area is
         * smaller than the 256MiB alignment. */
        if (guestArea < ppc64AlignSize) {
        (...)


Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>


>           virReportError(VIR_ERR_XML_ERROR, "%s",
>                          _("minimum target size for the NVDIMM "
>                            "must be 256MB plus the label size"));
> 

On Mon, 2020-12-07 at 16:18 -0300, Daniel Henrique Barboza wrote:
> On 12/7/20 2:07 PM, Andrea Bolognani wrote:
> > +++ b/src/qemu/qemu_domain.c
> > @@ -5363,7 +5363,7 @@ qemuDomainNVDimmAlignSizePseries(virDomainMemoryDefPtr mem)
> >       /* Align down guest_area. 256MiB is the minimum size. Error
> >        * out if target_size is smaller than 256MiB + label_size,
> >        * since aligning it up will cause QEMU errors. */
> > -    if (mem->size < (ppc64AlignSize + mem->labelsize)) {
> > +    if (guestArea < ppc64AlignSize) {
> 
> Makes sense. I suggest to simplify the comment right above it as well:
> 
>         /* Align down guest_area. We can't align down if guest_area is
>          * smaller than the 256MiB alignment. */
>         if (guestArea < ppc64AlignSize) {
>         (...)
> 
> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>

Thanks, I've updated the comment as you suggested and pushed the
change.

-- 
Andrea Bolognani / Red Hat / Virtualization