From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id E57161FF13B for ; Wed, 11 Feb 2026 12:22:00 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id E8964199E9; Wed, 11 Feb 2026 12:22:44 +0100 (CET) Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Wed, 11 Feb 2026 19:22:33 +0800 Message-Id: To: , "Thomas Lamprecht" Subject: Confirmation needed: IPCC.xs behavior with empty response data From: "Kefu Chai" X-Mailer: aerc 0.20.0 X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1770808872634 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.134 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: 5XNJ5CGKKY553SMGEC3JCURHQNSOVL4I X-Message-ID-Hash: 5XNJ5CGKKY553SMGEC3JCURHQNSOVL4I X-MailFrom: k.chai@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Hi PVE developers, I'm working on the Rust port of pmxcfs and discovered what appears to be an issue in the Perl XS binding (src/PVE/IPCC.xs) when handling IPC operations that return empty response data. I'd like to confirm whether this is expected behavior or a bug before proceeding with the fix. ## Context The ipcc_send_rec() function in IPCC.xs has the following logic (lines 157-= 167): ```c if (res_header->error < 0) { errno =3D -res_header->error; XSRETURN_UNDEF; } else { errno =3D 0; if (dsize > 0) { RETVAL =3D newSVpv(ipcbuffer + sizeof(struct qb_ipc_response_header= ), dsize); } else { XSRETURN_UNDEF; // Line 165: Returns undef when dsize =3D=3D 0 } } ``` The issue is at line 165: when an IPC operation succeeds (error >=3D 0) but returns no data (dsize =3D=3D 0), the function returns undef with errno=3D0. This is semantically incorrect - successful operations should return a defined value (even if empty string). ## Affected Operations This affects IPC operations that legitimately return empty responses on suc= cess: ### 1. SET_STATUS (op 4) C implementation in server.c (lines ~144-161): ```c } else if (request_id =3D=3D CFS_IPC_SET_STATUS) { cfs_status_update_request_header_t *rh =3D (cfs_status_update_request_h= eader_t *)data; int datasize =3D request_size - sizeof(cfs_status_update_request_header= _t); if (ctx->read_only) { result =3D -EPERM; } else if (datasize < 0) { result =3D -EINVAL; } else { rh->name[sizeof(rh->name) - 1] =3D 0; char *dataptr =3D (char *)data + sizeof(cfs_status_update_request_h= eader_t); result =3D cfs_status_set(rh->name, dataptr, datasize); // Note: result =3D 0 on success, no data appended to outbuf } } ``` ### 2. LOG_CLUSTER_MSG (op 7) C implementation in server.c (lines ~194-223): ```c } else if (request_id =3D=3D CFS_IPC_LOG_CLUSTER_MSG) { cfs_log_msg_request_header_t *rh =3D (cfs_log_msg_request_header_t *)da= ta; int datasize =3D request_size - G_STRUCT_OFFSET(cfs_log_msg_request_hea= der_t, data); int msg_len =3D datasize - rh->ident_len - rh->tag_len; if (ctx->read_only) { result =3D -EPERM; } else if (msg_len < 1) { result =3D -EINVAL; } else { char *msg =3D rh->data; if ((msg[rh->ident_len - 1] =3D=3D 0) && ...) { char *ident =3D msg; char *tag =3D msg + rh->ident_len; msg =3D msg + rh->ident_len + rh->tag_len; time_t ctime =3D time(NULL); clog_entry_t *entry =3D (clog_entry_t *)alloca(CLOG_MAX_ENTRY_S= IZE); if (clog_pack(entry, cfs.nodename, ident, tag, ctx->client_pid, ctime, rh->priority, msg)) { cfs_cluster_log(entry); } result =3D 0; // Success, no data appended to outbuf } else { result =3D -EINVAL; } } } ``` Response sending (server.c): ```c int resp_data_len =3D resp ? outbuf->len : 0; // resp_data_len =3D 0 for e= mpty response res_header.error =3D result; // error =3D 0 for success res_header.size =3D sizeof(res_header) + resp_data_len; // Send response with error=3D0, dsize=3D0 qb_ipcs_response_sendv(c, iov, iov_len); ``` ## Why Existing Code Doesn't Break The production code in PVE/Cluster.pm has a wrapper that "accidentally" works around this bug: ```perl my $ipcc_send_rec =3D sub { my ($msgid, $data) =3D @_; my $res =3D PVE::IPCC::ipcc_send_rec($msgid, $data); # Only die if result is undef AND errno !=3D 0 die "ipcc_send_rec[$msgid] failed: $!\n" if !defined($res) && ($! !=3D = 0); return $res; }; ``` When ipcc_send_rec returns undef with errno=3D0: - !defined($res) =3D TRUE - $! !=3D 0 =3D FALSE (errno is 0) - Combined: TRUE && FALSE =3D FALSE =E2=86=92 no exception thrown - The function silently returns undef, which callers ignore: ```perl eval { $ipcc_update_status->("kv/$key", $data) }; warn $@ if $@; # Only checks exception, not return value ``` So the bug is masked by: 1. The wrapper's "accidental" acceptance of undef with errno=3D0 2. Production code ignoring return values and only checking exceptions ## Test Verification However, proper test code that checks return values DOES fail: ```perl my $result =3D PVE::IPCC::ipcc_send_rec($op_code, $request); my $errno =3D $! + 0; if (defined $result) { test_success(""); } else { if ($errno =3D=3D 1) { # EPERM test_success("Permission denied"); } else { test_failure("Unexpected errno: $errno"); # FAILS HERE with errno= =3D0 } } ``` Testing with C pmxcfs: ```bash /usr/bin/pmxcfs -f & perl tests/ipc/perl/set-status.pl test-key 'test data' # Output: FAILED: Unexpected errno: 0 ``` ## Potential Fixes Option 1: Fix IPCC.xs (recommended) ```c } else { errno =3D 0; if (dsize > 0) { RETVAL =3D newSVpv(ipcbuffer + sizeof(struct qb_ipc_response_header= ), dsize); } else { RETVAL =3D newSVpv("", 0); // Return empty string instead of undef } } ``` This makes the semantics correct: success returns defined value, failure re= turns undef. **Option 2: Workaround in pmxcfs** Return a single space character instead of empty response. This is what I'v= e temporarily implemented in the Rust port. ## Questions 1. Should we fix IPCC.xs to return empty string for successful operations with no data? This makes the API more correct and consistent. 2. The wrapper in Cluster.pm would continue to work fine with this fix (empty string is still defined). 3. Are there any other callers outside pve-cluster that might depend on the current behavior? 4. For the Rust port: should we maintain bug-for-bug compatibility, or fix it properly? I lean toward fixing IPCC.xs since the current behavior is semantically wrong (success should not return undef), and the fix is unlikely to break existing code since the wrapper already handles both cases. Thanks for your guidance! Best regards, Kefu Chai